OpenColorIO icon indicating copy to clipboard operation
OpenColorIO copied to clipboard

[Python] `Colorspace.setReferenceSpaceType` missing or intended ?

Open MrLixm opened this issue 3 years ago • 4 comments

Context : PyOpenColorIO==2.1.0

Hello, When having a look at the Colorspace class I noticed that we can set the referenceSpace in instance's init but then, there is no method that allows you to modify it ( after instance creation ). Only Colorspace.getReferenceSpaceType is provided.

Is there a particular reason for this logic or is it a missing feature ?

This means that currently, to convert a "display colorspace" to a "regular colorspace", I have to create a new colorspace instance with the scene reference space type that's inherit from all the display colorspace attributes. Context is me trying to have a config both working with OCIO v1 and OCIO v2 and this complexity the process.

Cheers. Liam.

MrLixm avatar Apr 10 '22 16:04 MrLixm

I dunno if that's a design choice or not. Presumably, Display and Scene color spaces are defined relative to different spaces, so naively converting a scene color space to a display color space could have unintended consequences if the transforms aren't immediately changed accordingly as well; and without a-priori knowledge of the containing config, there isn't enough information for adjusting the transforms the automatically.

Are you trying to convert an OCIOv2 config to an OCIOv1-compatible config with PyOpenColorIO, or something? Given that OCIOv2 colorspaces may contain aliases, encodings, and categories, and may not contain allocation vars or bitdepths, I feel like the path of least resistance inevitably involves an intermediate reconciliation method for validating / converting color space attributes (to say nothing of the transforms)

zachlewis avatar Apr 19 '22 16:04 zachlewis

Hello, I'm trying to have a system to write a config that both works on v1 and v2, so of course, you specify additional information for v2 but they are not used on v1. I was simply subclassing PyOpenColorIO.ColorSpace right now for simplicity and I have a conform_to_version method that re-arranges the internals depending on the OCIO version. I can easily modify the transforms for example but to convert it to a regular colourspace this means I have to swap the current Display ColorSpace instance with a new ColorSpace instance :

pseudo-code :


class Colorspace(ocio.ColorSpace):
    ref_space = ocio.REFERENCE_SPACE_SCENE

    def conform_to_version(self, major, minor=0, patch=0) -> Optional[Colorspace]:
        pass

class ColorspaceDisplay(Colorspace):
    ref_space = ocio.REFERENCE_SPACE_DISPLAY

    def conform_to_version(self, major, minor=0, patch=0) -> Optional[Colorspace]:
        return Colorspace(...)

I have a solution but things could be simpler if I didn't have to create a new instance. So I could do this :

class Colorspace(ocio.ColorSpace):

    def __init__(self, ..., is_display):

        super(Colorspace, self).__init__(...)

        if is_display:
            self.setReferenceSpaceType(ocio.REFERENCE_SPACE_DISPLAY)
        else:
            self.setReferenceSpaceType(ocio.REFERENCE_SPACE_SCENE)

        return

    def conform_to_version(self, major, minor=0, patch=0):

        if major == 1:
            self.setReferenceSpaceType(ocio.REFERENCE_SPACE_SCENE)
            # modify transforms ...

But I do understand that there is still a logic for not having it as the user has to be aware of the change required to have it working.

Cheers.

MrLixm avatar Apr 19 '22 16:04 MrLixm

@MrLixm , this was by design. As Zach wrote, changing the reference space type would invalidate the transforms and some of the other attributes currently associated with a color space. There's no way the library would really be able to figure out what needed to change and how. So it is best for the user to start over, create a new color space of the desired reference type and then populate the transforms and attributes appropriately.

doug-walker avatar Apr 19 '22 23:04 doug-walker

Thanks for the answers, it is perfectly understandable. I still advocate for having it, evens if it performs a complete reset of the instance for the reasons mentioned. It is at least, in my opinion, more convenient to rebuild it fully using set-like methods than having to create a new class instance. Cheers.

MrLixm avatar Apr 20 '22 07:04 MrLixm