Respect display and view in ExporterReview - #347
Conversation
Update ExporterReview to handle display and view color data separately from colorspace. This ensures correct OCIO metadata when baking reviews via specific display/view transforms in Nuke.
jakubjezek001
changed the title
jakubjezek001
changed the title
jakubjezek001
added
the
type: enhancement
Check if config_data is valid before checking for display value to prevent errors when using older Nuke OCIO configurations.
| repre["colorspaceData"].pop("colorspace", None) | ||
| repre["colorspaceData"].update({ | ||
| "display": display, | ||
| "view": view, | ||
| }) |
There was a problem hiding this comment.
Would it be more explicit to adjust the colorspace data from the instance.data to collect display and view instead of doing in representation data?
There was a problem hiding this comment.
The most accurate would be if we always store it in representation display view I think, if we can. Because that way different representations can each have a different colorspace or display/view set instead of all representations sharing the one.
There was a problem hiding this comment.
@jakubjezek001 can you make a follow up issue to core to allows also setting display and view via set_colorspace_data_to_representation?
There was a problem hiding this comment.
We'll need to be wary of a few things @jakubjezek001
It looks like we're taking the display/view from the Viewer LUT and not from the write node. What makes it so in Nuke that the write node actually includes that display/view in the written out image? Or is that display/view NOT written into the pixel data of the image that Nuke writes out?
Because there's a few things we should consider:
- The output image may or may not have a display/view baked in, the source display
- The user did see in their scene a display/view that the output image may not get baked in but still wants to apply to reviewables, the scene display
- We may want to convert it to some display/view, the target display (although this is usually more up to e.g. Extract Review or alike to decide.)
See the terminology here: ynput/ayon-core#1541 (comment)
The question here then is, are we referring here to the Scene Display (what the user was seeing in Nuke through the viewer) or are we referring to the Source Display (what the image written out by Nuke got baked in.)
I'd say whenever we put data on representation as display/view it must really represent the source display/view. Putting scene display/view on the representation makes little scene I'd argue. We may also be referring to the "target display/view" but I haven't seen a case yet where we'd want that explicitly defined at this level PER representation perhaps?
@MustafaJafar just so you're aware ☝️ - we may want to make sure this terminology is clear in our docs somehow. And that perhaps certain settings like those for Extract OIIO Transcode or alike could perhaps use as well in settings so it's clear that you're setting either target display/view, etc.
Yes you are not completely wrong. The display/view is actually baked in pixels, that is why it is added to representation's colorspaceData so down the line it can be used as sourceDisplay/View. Here we are applying for backward compatibility with older nuke versions - where write node did not support display/view knobs, applying it via OCIODisplay node above the write node and setting to RAW colorspace just to disable any colorspace baking by the write node. |
BigRoy
left a comment
BigRoy
left a comment
There was a problem hiding this comment.
Not a crazy approach ;) so please validate it does what you'd expect in testing, and then fine with me.
thank you, but in the slate at ACES 1.3 this did not fix the issue. It seems the Display colorspace in ACES 1.3 are really correctly working as RRT and ODT and this was already provided by the code before the fixes. My openion is that this enhancement is still good to be merged since it makes more sense and is more explicit. But without it, nothing is broken. |
Let's go through that next week then please. |
4 participants
Changelog Description
ExporterReview now preserves OCIO display and view data separately when baking reviews in Nuke. This enables downstream integration to derive the correct colorspace template value for display/view transforms instead of losing the selected view metadata.
Additional info
Representation colorspace data now accepts
displayandview. The dependentayon-corechange derives the integration template value from these fields when no direct colorspace is available.Testing notes:
At your Intermediate baking preset use dispaly and view from your active OCIO config. Ideally test at ACES 1.3.
The outcome should look as expected.
Dependency
Depends on ynput/ayon-core#1983