Cycles Renderer : Fix auto-tiled renders#6945
Conversation
This fixes issues reported by users attempting Cycles renders with auto-tiling enabled, and also ensures Cycles' tile buffer images are written to a temp directory rather than `$GAFFER_ROOT/bin` and are cleaned up after use. Early [investigations](GafferHQ#5233 (comment)) went down the path of accumulating and combining the tiles in our output driver, but this falls short when it comes to denoising as the individual tiles sent to the output driver aren't denoised. The alternative used here is to connect to the Cycles session's `full_buffer_written_cb` callback, which provides the filename of the tile buffer image once all tiles have finished writing to the buffer. It appears the host is required to use this callback to then tell the session to post-process the buffer file (including performing full-frame denoising), after post-processing the session sends the output driver the whole frame as a single tile. Setting `ccl::BufferParams.window_width` and `window_height` is also necessary as these values are written into the metadata of the tile buffer EXR and used to determine the size of the final tile sent to the output driver, if these are left unset then the tile size defaults to (0, 0). Fixes GafferHQ#5233. Fixes GafferHQ#6767.
johnhaddon
left a comment
johnhaddon
left a comment
There was a problem hiding this comment.
Nice one! No idea why Cycles thinks we should be responsible for doing that, but it's great that it works. One comment about subTests but otherwise LGTM.
|
|
||
| for cropWindow in ( False, True ) : | ||
|
|
||
| with self.subTest( cropWindow = cropWindow ) : |
There was a problem hiding this comment.
What does it mean to nest subtests? Can we just have one subTest( tileSize = tileSize, cropWindow = cropWindow ) instead?
There was a problem hiding this comment.
All the Python docs say on the matter is "A test case can contain any number of subtest declarations, and they can be arbitrarily nested". In practice, nested or flat subtests appear to behave similarly, you see the same number of subtest combinations and equivalent failure reporting where the arguments from the outer subtest combine with the inner. I think the main benefit would be if you were asserting in the outer loop and also wanted that to run inside a subtest.
In this case nesting gives us no advantage over a single subtest and combining them would reduce the indentation, so I've done that in 0a8f159 along with fixes for the Windows build errors.
murraystevenson
force-pushed
the
cyclesAutoTiledRenderFix
branch
from
09c1458 to
0a8f159
Compare
2 participants
This fixes issues reported by users attempting Cycles renders with auto-tiling enabled, and also ensures Cycles' tile buffer images are written to a temp directory rather than
$GAFFER_ROOT/binand are cleaned up after use.Early investigations went down the path of accumulating and combining the tiles in our output driver, but this falls short when it comes to denoising as the individual tiles sent to the output driver aren't denoised. The alternative used here is to connect to the Cycles session's
full_buffer_written_cbcallback, which provides the filename of the tile buffer image once all tiles have finished writing to the buffer. It appears the host is required to use this callback to then tell the session to post-process the buffer file (including performing full-frame denoising), after post-processing the session sends the output driver the whole frame as a single tile.Setting
ccl::BufferParams.window_widthandwindow_heightis also necessary as these values are written into the metadata of the tile buffer EXR and used to determine the size of the final tile sent to the output driver, if these are left unset then the tile size defaults to (0, 0).Fixes #5233.
Fixes #6767.