Page MenuHome

Compositor automated testing
Needs ReviewPublic

Authored by Habib Gahbiche (zazizizou) on Nov 30 2019, 1:30 PM.

Details

Summary

Added support for compositor tests. Compositor tests can be added, executed and viewed in a similar way to cycles
and other render engines tests.

Running test:
ctest -R compositor

Updating test:
BLENDER_TEST_UPDATE=1 ctest -R compositor

Viewing test results:
typically saved under build_folder/tests/compositor/report.html


Attached compositor.zip must be extracted to lib/tests.A successful run should produce a report like the following:

Diff Detail

Repository
rB Blender
Branch
compositor_test (branched from master)
Build Status
Buildable 10553
Build 10553: arc lint + arc unit

Event Timeline

The patch is almost done, but I would appreciate some help regarding these issues:

  1. Blank render when blender is ran in background mode: Nodes are created and linked correctly but blender always renders a blank image. Saving the blend file (after running in background) shows no background image. Selecting the same input image (again!) seems to refresh the compositor and give correct results again
  2. Beautify tree not working: this issue is less important (see comment in line 279)
tests/python/modules/compositor_test.py
279 ↗(On Diff #19979)

This is always 0 for all nodes, I'm not sure why...

  1. Blank render when blender is ran in background mode:

Never mind, I just found out this is intentional: https://archive.blender.org/wiki/index.php/Dev:Ref/Release_Notes/2.67/Compositing_Nodes/
I will change the test to write the evaluated image on disk and then loaded again to compare with expected result.

Habib Gahbiche (zazizizou) planned changes to this revision.Dec 1 2019, 11:24 AM

Rendered images are now written to disk and read again to compare them with expected result.

The class CompositorTest does not check if the passed node tree is valid but this check is done when CompositorTest.run_test() is called.

Correct me if i'm wrong, but seems you are assembling the node setup in the code. This sounds like an unnecessary overhead, and also doesn't allow to catch possible issues in versioning code when node behaviour changes.

I would imagine such tests will be done similar to Cycles, where there is a set of pre-defined .blend files which are rendered with the minimal required resolution and number of samples.
The script can be used to make an initial set of files. But having files helps maintaining the test, opening when it's needed to investigate what's going on and things like that.

Thanks for the feedback.

Correct me if i'm wrong, but seems you are assembling the node setup in the code. This sounds like an unnecessary overhead, [...]

It was intentional to use code to assemble a node tree. The idea behind it is to make it possible to generate a test case using one line of code (e.g. compositor.py line 35. I tried to explain the idea behind the framework in T71834.

and also doesn't allow to catch possible issues in versioning code when node behaviour changes.

These are meant to be regression tests, so if a node behavior changes, (e.g. output or API change) then the test will fail, since it always compares the rendered image with a reference image.

I would imagine such tests will be done similar to Cycles, where there is a set of pre-defined .blend files which are rendered with the minimal required resolution and number of samples.
The script can be used to make an initial set of files. But having files helps maintaining the test, opening when it's needed to investigate what's going on and things like that.

I thought it would be harder to maintain .blend files (which are versioned with SVN) than a script (versioned with git). Either way, the script that generates the tests has to be maintained, which comes down to the same effort.
Also, here, no blend files are needed at all to run the test. The blend file ${TEST_SRC_DIR}/compositing/compositor.blend is only used to display the test's results if the user wishes to inspect a test.

In any case, I'm willing to change the approach to use blend files if that aligns better with other blender tests. Is the rest of the approach otherwise alright?

I thought it would be harder to maintain .blend files (which are versioned with SVN) than a script (versioned with git). Either way, the script that generates the tests has to be maintained, which comes down to the same effort.

In the approach i was describing there is no need to maintain the script: it just takes care of spending an initial time creating a lot of similar .blend file. This is what i did to create an initial set of Cycles regression tests.

Also, here, no blend files are needed at all to run the test. The blend file ${TEST_SRC_DIR}/compositing/compositor.blend is only used to display the test's results if the user wishes to inspect a test.

That isn't a great approach to automated tests indeed.
Did you see the way how we do regression test for Cycles? It was also used for Eevee (although, due to specifics of render on GPU those are not enabled by default yet). I would imagine Compositor can use same exact approach.

To me it seems easier to understand what's going on when you can actually open .blend file and inspect it rather than trying to visualize the graph in your mind when reading code.

Did you see the way how we do regression test for Cycles? It was also used for Eevee (although, due to specifics of render on GPU those are not enabled by default yet). I would imagine Compositor can use same exact approach.

No, but I will have a closer look at those tests and see what I can reuse for compositor. Thanks for the hint.

To me it seems easier to understand what's going on when you can actually open .blend file and inspect it rather than trying to visualize the graph in your mind when reading code.

Might be true for complex node trees (then you could just execute the test with the option to open blender for inspection), but for simple one node tests, you only have to read the node's name and its parameters (so 1 line of "code").

But first I will look at Cycles tests :)

Habib Gahbiche (zazizizou) planned changes to this revision.Dec 2 2019, 8:01 PM

I'm having a hard time trying to reuse the testing and reporting files for cycles and eevee. I get the error 'No render result found' and I don't know how to generate the renders, so I could really use some help here.

So far I tried the following:

  1. wrote [[ https://developer.blender.org/differential/changeset/?ref=232452 | compositor_test.py ]] similar to cycles_render_test.py
  2. call the script using the following command (no full paths for simplicity):
compositor_render_tests.py "-blender" "MacOS/Blender" "-testdir" "lib/tests/compositing_new_test_src_dir/zmask/" "-idiff" "/lib/darwin/openimageio/bin/idiff" "-outdir" "/Users/habibgahbiche/blender-git/build_darwin/tests/compositing_new_out_dir/"

and the error I get is 'No render result file found'. Cycles test seems to work even though no previous renders are available.

Habib Gahbiche (zazizizou) edited the summary of this revision. (Show Details)

I changed the approach completely. Please let me know if you think this is the right way to test the compositor. If so, I will invest more time in generating test cases (should get more meaningful, faster, more representative of real world usage etc..).

Habib Gahbiche (zazizizou) edited the summary of this revision. (Show Details)

Added more tests

Overall the patch looks good to me. Still need to perform a test run.

tests/python/CMakeLists.txt
642

would use flip_scale_rotate as naming convention. Similar to cycles.

Habib Gahbiche (zazizizou) marked an inline comment as done.
Habib Gahbiche (zazizizou) edited the summary of this revision. (Show Details)

Addressed comments from code review:

  • changed test naming convention

Added a few more tests:

  • compositor_bokeh_mask_test
  • compositor_color_ramp_colorize_test
  • compositor_distorted_color_key_test

code wise it seems fine and works as expected.

  • I would keep the textures smaller. No need to allocate so much diskspace for the test. PNG's of 256x256 should already be sufficient to perform the tests. For first test I would propose to have a test-file per node and for certain use cases to add more global test.
  • I would propose to move forward to just add a few node-based tests and put this in master. From there on we can build up the test suite.

Thanks for the feedback, @Jeroen Bakker (jbakker)

  • I would keep the textures smaller. No need to allocate so much diskspace for the test. PNG's of 256x256 should already be sufficient to perform the tests. For first test I would propose to have a test-file per node and for certain use cases to add more global test.

The same image is used for all files, so only around 300kb of disk space is required for the first test and none for further tests. I didn't look into execution speed though, so I'll see if tests can ran faster.

  • I would propose to move forward to just add a few node-based tests and put this in master. From there on we can build up the test suite.

Do you mean create a test for each single compositor node?
From looking at recent bug reports I thought real world examples are more valuable so I started with that. But I'll be happy to add tests for all nodes (probably using a script or something)

We now have just a small number of test cases. But the idea is to grow. For cycles and eevee we did the same that the image it is compared against is for a small resolution otherwise the source repository will grow to fast when we need to update the images. It is fine to have a good set of mix cases. Most of the time we export them from existing reports that fail. The issue with real world examples is that you can find out that it fails, but it doesn't point the developer into the direction why it failed.

I would suggest to make sure to use small images as that is a technical problem and then we could iterate on when this patch has landed.

Thanks for your work so far!

Brecht Van Lommel (brecht) added inline comments.
tests/python/compositor_render_tests.py
15–18

Is this necessary? We have it for Eevee because the .blend files are shared with Cycles, and because we have a large number of existing files that would need changes.

But since these are all new .blend files, it's better if opening the .blend file gives the same result as the test.