Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ocioview updates & fixes #1966

Merged

Conversation

michdolan
Copy link
Collaborator

@michdolan michdolan commented Apr 17, 2024

This PR makes a number of updates and improvements to ocioview:

  • When closing the application or creating a new config, users will now only be prompted to save their config if changes have been made. Previously the initial application state would be detected as a modified config. Note: config modification detection is limited to the data included in the config's cache ID, which does not currently include the config's name or description.
  • Per suggestion from @KelSolaar in ocioview image and PySide6 updates #1912 , changed the message router interface to use properties instead of getters and setters.
  • Added a warning when loading a config that contains a view definition that references a view transform and non-display color space, which results in that view being interpreted as a OCIO v1 style view, dropping the view transform. Technically OCIO supports a view with a view transform and a scene reference color space, but in ocioview we intentionally limit color space selection to display color spaces when using a view transform.
  • Improved consistency of icon sizing throughout application, and improved pixmap transformation quality for consistent icon filtering across tabs, buttons, and items.
  • Added ProcessorContext interface to reference input and output processor parameters so application components that receive a viewer's processor can inspect the config items that created it. This dataclass includes the processor's: input color space name, output transform item type, output transform item name, and transform direction. This should resolve questions raised in PR: ocioview - Chromaticities Inspector #1914 .

@michdolan michdolan changed the title ocioview UX improvements ocioview updates & fixes Apr 18, 2024
@@ -156,14 +169,19 @@ def start_routing(self) -> None:
self._handle_config_message(msg_raw)

# OCIO processor
elif isinstance(msg_raw, ocio.Processor):
self._prev_cpu_proc = msg_raw
elif (
Copy link
Contributor

@KelSolaar KelSolaar May 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit "magical" :)

Might need a dedicated definition in the future to manage all those comparisons if we need to do it elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about this block specifically:

                isinstance(msg_raw, tuple)
                and len(msg_raw) == 2
                and isinstance(msg_raw[0], ProcessorContext)
                and isinstance(msg_raw[1], ocio.Processor)

Copy link
Contributor

@KelSolaar KelSolaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @michdolan, LGTM!

@KelSolaar KelSolaar merged commit 7e91b0e into AcademySoftwareFoundation:main May 6, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants