Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Colorspace Management and Distribution #4195

Conversation

jakubjezek001
Copy link
Member

@jakubjezek001 jakubjezek001 commented Dec 8, 2022

Brief description

Adding colorspace management

Description

Adding colorspace management functionality. Settings providing now to each host and global to set ImageIO config and file rules. Publishing is adding colorspace to each representation data so during loading we can maintain colorspace across all hosts connected to OpenPype. In case there is no colorspace saved on representation file rules could be used and define colorspace per file path.

Additional info

The plan is to integrate each pixel data representation with color space data in representation_db document. So loaders could set correct colorspace used during publishing. Here is the example of colorspace data.

{
    "colorspace": "linear",  # for other publish plugins and loaders
    "configData": {
        "path": "/abs/path/to/config.ocio",  # for future references in case need
        "template": "{project[root]}/path/to/config.ocio"  # for other plugins within remote publish cases
    }
}

How to integrate it to your host

  1. Each host setting schema should have following shemas
{
    "key": "imageio",
    "type": "dict",
    "label": "Color Management (ImageIO)",
    "is_group": true,
    "children": [
        {
            "type": "schema",
            "name": "schema_imageio_config"
        },
        {
            "type": "schema",
            "name": "schema_imageio_file_rules"
        }

    ]
}
  1. Use any mechanism to set OCIO config to host app resolved from openpype\pipeline\colorspace.py:get_imageio_config

    • either set OCIO environment during host launching via pre-launch hook
    • or to set workfile ocio config path if host api is available
  2. Each pixle related exporter plugins has to use parent class openpype\pipeline\publish\publish_plugins.py:ExtractorColormanaged and use it similarly as it is already implemented here openpype\hosts\nuke\plugins\publish\extract_render_local.py

  • get_colorspace_settings: is solving all settings for the host context
  • set_representation_colorspace: is adding colorspaceData to representation. If the colorspace is known then it is added directly to the representation with resolved config path.

Documentation

Using OCIO config

Each path input is supporting anatomy template key formating and also environment variables. Default distributed value is set to {OPENPYPE_ROOT}/vendor/bin/ocioconfig/OpenColorIOConfigs/aces_1.2/config.ocio. If any of host config is enabled and set to any path, global config path is overriden.

Config path can be defined to particular shot target with following path input {root[work]}/{project[name]}/{hierarchy}/{asset}/config/aces.ocio

Using File rules

File rules are inspired by OCIO v2 configuration (more info here). Since each rule is having unique name it can be overidden by host activated File rules (example: project_settings/nuke/imageio/file_rules/rules). Pattern input is using REGEX expression syntax (try regexr.com). . Matching rules procedure's intention is to be used during publishing or loading of representation. Since the publishing procedure is run before integrator formates publish template path, make sure the pattern is working or any work render path.

Colorspace name value is string input and no validation is run after saving of changes. We recommand to open a config.ocio file and search exact colorspace names for validation.

@jakubjezek001 jakubjezek001 self-assigned this Dec 8, 2022
@ynbot
Copy link
Contributor

ynbot commented Dec 8, 2022

@jakubjezek001 jakubjezek001 added the type: feature Larger, user affecting changes and completely new things label Dec 8, 2022
…ribution' into feature/OP-4404_color-v3-Settings-global-config-with-mapping-and-file-rules
…ngs-global-config-with-mapping-and-file-rules

Global: adding imageio to settings
…ribution' into feature/OP-4405_color-v3-abstraction-color-mapping-and-file-rules
@ynbot
Copy link
Contributor

ynbot commented Jan 16, 2023

Task linked: OP-4406 color-v3: global color plugins

@jakubjezek001
Copy link
Member Author

I would imagine that if you have project_settings/nuke/imageio/ocio_config/enabled enabled then we can assume this as using OCIO with a custom path.

Yes your assumption is right, plan was to integrate hosts in next PR. But I had included the Nuke host in this PR. You can just enable OCIO config and add paths like in the example image.
image

@tokejepsen
Copy link
Member

I would imagine that if you have project_settings/nuke/imageio/ocio_config/enabled enabled then we can assume this as using OCIO with a custom path.

Yes your assumption is right, plan was to integrate hosts in next PR. But I had included the Nuke host in this PR. You can just enable OCIO config and add paths like in the example image. image

I think if this is to be merged, the Nuke implementation needs to be sorted. Currently if you enabled the new OCIO switch, the code wont use this. The new OCIO should be queried first for existence and have precedence over the old settings.

@jakubjezek001
Copy link
Member Author

I think if this is to be merged, the Nuke implementation needs to be sorted. Currently if you enabled the new OCIO switch, the code wont use this. The new OCIO should be queried first for existence and have precedence over the old settings.

Perhaps I wasn't clear enough, but this is already merged. Please pull latest changes and try it.

Comment on lines +157 to +168
# match file rule from path
colorspace_name = None
colorspaces = get_ocio_config_colorspaces(config_path)
for colorspace_key in colorspaces:
# check underscored variant of colorspace name
# since we are reformating it in integrate.py
if colorspace_key.replace(" ", "_") in path:
colorspace_name = colorspace_key
break
if colorspace_key in path:
colorspace_name = colorspace_key
break
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like quite an unreliable parsing method. I feel like this should either use file rules or don't try this at all - or the logic should be made way more explicit.

A filepath with aces or raw anywhere in the path will match, but there might be other colorspaces that contain aces but are longer?

Some examples of those conflicts could be:

acesproxy
acesproxy_ap1
acescct
acescct_ap1
rlf_rc
rlf_rc2
rlf_rc3
rlf_rc4
out_rec709
out_rec709d60sim
Input - Sony - Linear - S-Gamut
Input - Sony - Linear - S-Gamut Daylight
Input - Sony - Linear - S-Gamut3
Input - Sony - Linear - S-Gamut3.Cine

I've just done a quick search through OCIO 1.0.3 where it could fail to find the right colorspace.

There's a couple of things we could try to do to perform better matching.

  • Match only the filename, not the full path
  • Take the LONGEST match that is found
  • Or be even more explicit and e.g. ALWAYS expect an underscore before the colorspace or be in a certain location of the fname (e.g. before the extension, or before a dot, etc.). This is likely what'll make it most explicit.

Copy link
Member Author

@jakubjezek001 jakubjezek001 Jan 19, 2023

Choose a reason for hiding this comment

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

  • Match only the filename, not the full path
  • Take the LONGEST match that is found
  • Or be even more explicit and e.g. ALWAYS expect an underscore before the colorspace or be in a certain location of the fname (e.g. before the extension, or before a dot, etc.). This is likely what'll make it most explicit.

Yes that is very good point. Since it would be practical to keep full path, because some productions will want to keep colorspace at folder level - and it is almost impossible to parse only part of name - we have only option to evaluate the longest match.

I have add it into the next issue. I would address it in next patch release or do you suggest this feature will not be complete without this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not too bothered it being broken for now - the parsing will be somewhat unreliable no matter how far we get. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we could at least try, but it will never be optimal for all, yeah.. Thanks man!

@jakubjezek001 jakubjezek001 merged commit 8ed932b into release/3.15.x Jan 19, 2023
@github-actions github-actions bot added this to the next-minor milestone Jan 19, 2023
@ynbot
Copy link
Contributor

ynbot commented Jan 19, 2023

from openpype.pipeline import publish


class ExtractColorspaceData(publish.ExtractorColormanaged):
Copy link
Collaborator

@BigRoy BigRoy Jan 27, 2023

Choose a reason for hiding this comment

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

Warning

This PR has broken publishing in Maya for a project that has NO color space config set due to this ExtractColorspaceData node.

I cannot publish e.g. a pointcache instance:

Traceback (most recent call last):
  File "S:\openpype\OpenPype\.venv\lib\site-packages\pyblish\plugin.py", line 522, in __explicit_process
    runner(*args)
  File "S:\openpype\OpenPype\openpype\plugins\publish\extract_colorspace_data.py", line 46, in process
  File "S:\openpype\OpenPype\openpype\pipeline\publish\publish_plugins.py", line 376, in set_representation_colorspace
    colorspace_settings = self.get_colorspace_settings(context)
  File "S:\openpype\OpenPype\openpype\pipeline\publish\publish_plugins.py", line 331, in get_colorspace_settings
    anatomy_data=anatomy_data
  File "S:\openpype\OpenPype\openpype\pipeline\colorspace.py", line 365, in get_imageio_config
    "No OCIO config found in settings. It is "
FileExistsError: No OCIO config found in settings. It is either missing or there is typo in path inputs

This also goes to show this Extractor is INVALID.

Because additonally: it shouldn't enforce injecting colorspaceData for representations which are NOT a image/video/pixel format like e.g. a pointcache.
Plus it shouldn't fail if nog config is set.

Also FileExistsError sounds like it's exactly the opposite of what is actually occurring here, no?

@BigRoy BigRoy mentioned this pull request Jan 27, 2023
65 tasks
@mkolar mkolar deleted the feature/OP-2479_color-v3-Colorspace-management-and-distribution branch June 20, 2023 11:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Larger, user affecting changes and completely new things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants