ynput / ayon-core

Apache License 2.0
31 stars 36 forks source link

Color Management: Use profiles to determine OCIO path #490

Closed iLLiCiTiT closed 5 months ago

iLLiCiTiT commented 6 months ago

Changelog Description

Added more options to define OCIO path. Simplified using builtin paths to enumrator, kept custom paths, and added option to use published ocio file. All of that based on profile system. The profile system might help to remove imageio settings from host integrations in future.

Additional info

Modified settings to have profiles in core settings instead of list of filepaths. Another change is that result of a profile is single path instead of list of filepaths. For published representation is used product name filter and hardcoded representation name "ocioconfig". Latest version of the product is used.

A new function get_imageio_config_preset was added as replacement of get_imageio_config, new function was added as has different signature, it now expects folder path and task name. That was already required by the previous implementation, but the only way how to pass it to the function was through "anatomy data". A helper function get_current_context_imageio_config_preset was implemented to simplify most of the places where the function is used.

It was necessary to change signature of get_imageio_file_rules_colorspace_from_filepath and get_colorspace_name_from_filepath, they both had optional config_data argument which is now required, all places using the functions were modified.

Questions

  1. Should we add product type to filtering of the representation?
  2. Representation "ocioconfig" is a first shot idea, at this moment we don't have any logic creating them, but once we start it will be hard to change so we should think if the name is ok.
  3. Changing get_imageio_file_rules_colorspace_from_filepath and get_colorspace_name_from_filepath signature did break backwards compatibility. But the functions, as far as I know, were used only in ayon core. Should I kept backwards compatibility and use get_current_context_imageio_config_preset with warning in case it is not passed?

Testing notes:

I don't know, @jakubjezek001 please fill.

ynbot commented 6 months ago

Task linked: AY-5002 OCIO hook add settings presets published config

BigRoy commented 6 months ago

Just want to remark without going through all the code yet that this should likely come with a big "backwards incompatible" disclaimer. This change will invalidate any custom OCIO settings currently defined by a studio since the attributes have changed tremendously.

jakubjezek001 commented 6 months ago

The ocioconfig representation has been developed. Currently, we are only implementing product name validation. This is because the representation may originate from either the effect product type in Hiero Editorial or the ociolook product type in Traypublisher.

jakubjezek001 commented 6 months ago

I just saw a traceback in the server log that could be related to our issue. It seems like overrides that haven't changed are getting stuck in the settings. The server isn't fixing overrides if they don't differ from the built-in defaults. @iLLiCiTiT @martastain

Traceback (most recent call last):
      File "/backend/ayon_server/addons/addon.py", line 293, in get_studio_overrides
        return await target_addon.convert_settings_overrides(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/addons/core/0.3.1-dev.1/server/__init__.py", line 20, in convert_settings_overrides
        self._convert_imagio_configs_0_3_1(overrides)
      File "/addons/core/0.3.1-dev.1/server/__init__.py", line 33, in _convert_imagio_configs_0_3_1
        filepath = ocio_config["filepath"]
                   ~~~~~~~~~~~^^^^^^^^^^^^
    KeyError: 'filepath'
iLLiCiTiT commented 5 months ago

I just saw a traceback in the server log that could be related to our issue. It seems like overrides that haven't changed are getting stuck in the settings. The server isn't fixing overrides if they don't differ from the built-in defaults. @iLLiCiTiT @martastain

Good spot, fixed.