OCIO: don't discard resolved view when pipeline instance has no display (thumbnails / offscreen renders)#301
Open
jhery-rdo wants to merge 2 commits into
Conversation
… double-grade) The dedicated 'thumbnail_processor' OCIO instance has no Display set (never attached to a viewport/media source). OCIOEngine::process_thumbnail's combined `if (display.empty() or view.empty())` reset _view to the config default view (Client-look), so raw / display-referred media (e.g. dailies MOVs) got a double grade in thumbnails. Fill display and view independently so a valid caller-supplied view (e.g. "raw") survives an empty display, and use _view for the to-linear step. Complements 3d8678c3 (sws colorspace/range for the thumbnail decode): raw view + range-correct decode = the baked look, matching the viewport. (cherry picked from commit 4a0d5c7697939162bb30b473bac0777573ce47cf) Signed-off-by: Julien Hery <julien.hery@yahoo.fr>
…ers) Same defect as the process_thumbnail fix (4a0d5c76), second site: display_transform() reset both display AND view to the config defaults whenever either was empty. Offscreen/snapshot viewport pipeline instances always have an empty Display attribute, so a valid resolved view (e.g. 'raw' for un-tone-mapped media) was replaced by the default Client-look view, double-grading annotation exports and any offscreen render. Fill display and view independently instead. (cherry picked from commit e53a9849fc764cb25b25d2f05120cc30d7511b47) Signed-off-by: Julien Hery <julien.hery@yahoo.fr>
f202033 to
1c442d9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes thumbnails and offscreen renders being processed with the wrong OCIO view whenever the colour pipeline instance has no display attached.
OCIOEngine::process_thumbnail()andOCIOEngine::display_transform()reset both display and view to the config defaults when either is empty, so the correctly resolved per-media view is silently discarded for those instancesuntonemapped_view(e.g. display-referred dailies MOVs) get the config's default view applied on top of their baked-in look — a double grade — in thumbnails (media list, notes panel) and offscreen renders (snapshots, annotation exports). The interactive viewport is unaffected because its Display attribute is populated, which makes the defect easy to missChanges
src/plugin/colour_pipeline/ocio/src/ocio_engine.cpp: inprocess_thumbnail()anddisplay_transform(), fill display and view independently — substitute the config default only for the component that is actually empty, so a valid caller-supplied view survives an empty display (and vice versa). Inprocess_thumbnail(), also pass the filled_viewtomake_to_lin_processor()instead of the original (possibly empty)view, keeping the to-linear and display halves of the transform consistentTesting
untonemapped_viewfor display-referred QuickTimes: media-panel thumbnails, notes-panel thumbnails and annotation exports now render with the resolved per-media view instead of the config defaultdevelop(the affected code is identical in both)