Page MenuHome

Cycles: Added Cryptomatte passes.

Authored by Stefan Werner (swerner) on Jul 12 2018, 10:37 PM.
"Burninate" token, awarded by jonimercado."Love" token, awarded by monio."Love" token, awarded by maxon."100" token, awarded by Scaredyfish."Love" token, awarded by Tanguy."Love" token, awarded by vitorbalbio."Love" token, awarded by SteffenD."Love" token, awarded by Noss."Love" token, awarded by aliasguru.



This patch introduces Cryptomatte output passes to Cycles.

There are two modes:
"Accurate" mode, which is CPU only, tracks all
IDs encountered in a pixel before it sorts them by coverage and strips
any that exceed the number of selected Cryptomatte layers. Since that
relies on dynamic memory allocations, it is CPU only.
The other mode keeps track of no more IDs than the requested number of
layers. When the number of IDs for a given pixel exceed the number of
layers, it is possible that an ID with a higher coverage gets rejected
in favor of an ID with lower coverage, if the latter was sampled first.
It would be good improvement in the future to allow GPU renders too to
track any number of IDs per pixel, regardless of the number of
Cryptomatte layers.

In order to support a user-selectable number of Cryptomatte layers,
Cycles passses are now not only distinguished by type but also by name.
That way, any number of passes of type PASS_CRYPTOMATTE can be added
to a render. Going forward, this could become useful for other types too,
should we introduce light path expressions or custom AOVs.

Diff Detail

rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Sergey Sharybin (sergey) requested changes to this revision.Jul 20 2018, 12:19 PM
Sergey Sharybin (sergey) added inline comments.

string_startswith() ?




std::min -> this is what util_algorithm.h is for.




Include statements are supposed to be sorted alphabetically. See render/buffers.h above.


I'm sure this madness of ifdef is easily avoidable.


First you go away from string to char* when calling this function, then you go away from string to char* here in the pass? Whats' the point? Just compare string to string.



Stefan Werner (swerner) marked 8 inline comments as done.
  • Cycles: Cryptomatte writing now uses atomics on GPUs to avoid race conditions.
  • Cycles: Code style, fixed typos moving murmurhash to its own cpp file.

I think I found a way to write id passes using atomics.

  • Cycles: Compile fixes for OpenCL kernel.

I benchmarked the Victor scene at 50% size, all Cryptomatte options enabled.
Compiled with Visual Studio 2015 running on 2x Xeon E5-2660v2, that is 40 threads.

Accurate mode: 8m21s
Stochastic mode: 8m18s
Cryptomatte off: 8m10s

So STL appears to be a measurable overhead, but it is below 1% of total render time for this scene. A custom linked list from a memory pool still sounds good though, since that could also be a way of getting the accurate mode for GPUs.

I think I found a way to write id passes using atomics.

That seems like it would work, as long as there are enough free ID slots. If there are not enough it's random which object ID get skipped, but I guess that is acceptable for now.

Without resorting to dynamic memory allocations inside the kernel or grossly over allocating memory, there will always be a chance of randomly skipped data.

Without resorting to dynamic memory allocations inside the kernel or grossly over allocating memory, there will always be a chance of randomly skipped data.

I mean random in that the results could be different between renders, so the result is not exactly repeatable. A solution could be to write the minimum sample number next to each ID and prioritize lower samples, but I'm not sure it's worth it in practice.

Stefan Werner (swerner) marked 6 inline comments as done.
  • Merge branch 'master' of into cycles_cryptomatte
  • Merge branch 'master' of into cycles_cryptomatte
  • Cycles: Implemented full sorting for Cryptomatte layers for all possible renderign devices.
  • Cycles: Refactoring to remove duplicate Cyryptomatte id pass code.
  • Cycles: Addressed some of Sergey's comments on D3538
  • Cycles: Stochastic cryptomatte writing now adds coverage to last ID when it runs out of slots.

Addressed some of Sergey's comment.s

  • Cycles: A little refactor of the cryptomatte ID pass writting to reduce #ifdef clutter
  • Cycles: Removed PathState::matte_weight, using throughput instead for Cryptomatte

Some more inline comments.


Since CAS always returns the old value, we could actually detect the case that another thread already allocated this slot for the same ID here and get rid of the "--slot;" part.


For consistency, I'd prefer to also test for slot ID == ID_NONE here.


Could we move 2 * into the macro? It's always the same and it would make this part easier to read.

560 ↗(On Diff #11499)

I think this change can be removed?


Why is the sorting part of the kernel? I'd prefer to just move it to buffers.cpp, that would simplify the code a lot.

Also, sample is uninitialized if work_index >= total_work_size.


The code here could be deduplicated by having a wrapper function (or maybe function pointer?) that calls either flatten_buffer or sort_buffer.

Stefan Werner (swerner) marked 12 inline comments as done.
  • Cycles: More Cryptomatte code review changes.
  • Cycles: Skip calls to CPU kernel coverage buffer when accurate cryptomatte layers are turned off.
Stefan Werner (swerner) marked 2 inline comments as done.Aug 17 2018, 10:17 PM
Stefan Werner (swerner) marked 2 inline comments as done.

Looks good to me except for one detail.


This should check .x instead of .y I guess? In that case, the assert needs to be changed to .y == 0.0f.

Stefan Werner (swerner) marked an inline comment as done.
  • Cycles: Fixed typos in Crytpomatte code.
  • Fixed indentation.
Sergey Sharybin (sergey) requested changes to this revision.Aug 21 2018, 5:16 PM
Sergey Sharybin (sergey) added inline comments.

Narrowing from int. Is really needed to be unsigned short ?


Move to a function, call from the if() statement above.


This is quite scary.


Need update? Is being updated? A function pointer to function which updates the ray?


Clearly, only scale y and w. Without comment ;)


The convention is to:

  • Have own include first.
  • System includes.
  • Rest of includes, in alphabetical order.

There is pair in ccl namespace, see util_map.h.


Either use int for pixel_index or do a cast of tile.w to`size_t` to make math happen in size_t. Currently it's happening in int.


Why this index is in int?


Can we typedef unordered_map<float, float> for the clarity.

Will shorten so much code!


Make a named enum (ideally), and do not use use_ for not-a-boolean.


Just int.


Can we have this file more cycles-styled?

This revision now requires changes to proceed.Aug 21 2018, 5:16 PM
Stefan Werner (swerner) marked 9 inline comments as done.
  • Cycles: Addressed Sergey's cryptomatte omments.
  • Cycles: Fixed OpenCL build
  • Cycles: Code styling
Stefan Werner (swerner) marked 2 inline comments as done.
  • Cycles: Code styling
  • Merge branch 'cycles_cryptomatte' of into cycles_cryptomatte

Some more updates, addressing Sergey's comments.


Moving it to buffers.cpp excludes it from GPU acceleration.

Putting it in buffers.cpp isn't straight forward. RenderBuffers is agnostic of total sample count, and sorting every time get_pass_rect is called is wasteful. RenderTile knows about total sample count but is just a structure of data without methods. Turning it into its own RenderTile::Task (similar to denoising) seems like overkill to me.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 28 2018, 10:52 AM
This revision was automatically updated to reflect the committed changes.