Page MenuHome

Color Management panel naming
AbandonedPublic

Authored by George Vogiatzis (Gvgeo) on May 2 2019, 6:32 PM.

Details

Reviewers
Brecht Van Lommel (brecht)
Group Reviewers
User Interface
Summary

T61555 Color Management panel naming

  1. Rename view transform default option to Standard.
  2. Remove redundant Base contrast from look menu. And rename None option to None (Medium Contrast).
  3. Rename Film - Base Contrast to Film - Medium Contrast in config.ocio
  4. Remove unused variable viewindex2 from code.

Diff Detail

Repository
rB Blender

Event Timeline

Change to None (Medium contrasts).
From what I understand 'None' is important. And changed to Medium because, it matches better with the Medium High and Medium Low contrast options.

source/blender/imbuf/intern/colormanagement.c
563

Hit a wall here.
There is already an empty function

OCIO_ConstLookRcPtr *FallbackImpl::configGetLook(OCIO_ConstConfigRcPtr * /*config*/,
                                                 const char * /*name*/)
{
  return NULL;
}

but returns an empty struct (only an unused int inside).
Unlike the View Transform options.

const char *FallbackImpl::configGetView ( OCIO_ConstConfigRcPtr * /*config*/, const char * /*display*/, int index))
{
  if (index == 0) {
    return "Standard";
  }
  return NULL;
}

Not sure what the best option is, opted for a simple solution

Brecht Van Lommel (brecht) requested changes to this revision.May 2 2019, 7:00 PM

We need a solution for backwards compatibility of existing files.

It's a bit tricky since we don't actually know which OCIO transform the file was created for, it might have been a custom one. It's should be safe to convert if "default" does not and "Standard" does exist in the current OCIO config.

release/datafiles/colormanagement/config.ocio
375

I think this should be None instead of None (Medium Contrast). Where possible we want the equivalent looks to have the same name for all displays and view transforms.

source/blender/imbuf/intern/colormanagement.c
564

This only really makes sense for the builtin OCIO config, not the other ones. Even then I'm not sure it's needed.

This revision now requires changes to proceed.May 2 2019, 7:00 PM
George Vogiatzis (Gvgeo) updated this revision to Diff 15087.

Add backwards compatibility for existing files.

Improve config.ocio handling.

gez requested changes to this revision.May 4 2019, 7:30 PM
gez added a subscriber: gez.
This comment was removed by gez.
This revision now requires changes to proceed.May 4 2019, 7:30 PM
This revision now requires review to proceed.May 4 2019, 7:34 PM

Please leave the requests changes for developers that maintain the code, to keep it clear who can officially approve or reject it.

I have no problem with having Filmic Log available as a view, but it's separate from this patch. I intentionally changed the behavior from the original Filmic config and still think the way it works in Blender now follows the OpenColorIO design better, changing that is not on the table here.

gez added a comment.May 4 2019, 7:52 PM
This comment was removed by gez.

now follows the OpenColorIO design better

As someone on the OCIO Slack channel, engaged in the OCIO meetings, and discussing the development: You are wrong here.

Full stop.

Display is a colorimetric designation of a type of device.

View is a colorimetrically designed result for the Display target.

Look adjusts a view to a given aesthetic output.

Your opinion is one matter, but please stop trying to cite “OpenColorIO design.” It’s false.

We've had this discussion before. I wasn't convinced by your arguments then and I have no intent to revisit it, I stand by what I said.

This isn’t about your opinion beyond how you wish something to be in Blender. That's fine. It's a personal misunderstanding as to how OpenColorIO works, and pixel management in general.

This is about you speaking out of line regarding OpenColorIO design.

Again, to be clear, you are incorrect in your understanding.

As an addendum, if you do intend to adhere to some Blender rules about how Blender thinks colour transforms work, you'd be wise to follow OpenColorIO's design and not use children of GroupTransforms to accomplish it. Use the View, and append the look onto the view using suitable syntax. As an example:

- !<View> {name: Filmic Base Contrast, colorspace: Filmic Log Encoding, look: +Base Contrast}

I can't have an effective discussion with you, like in T63163 you're making authoritative claims without backing it up with evidence.

The SPI and ACES configurations don't even contain any looks and have Log available as its own display or view transform, while the Filmic config relies on looks to work. Maybe I am wrong and there's some information I'm not aware of, but I want to actually see it then.

This is a relatively minor user interface thing anyway, I'm not saying there's something wrong with the Filmic config. I just think the way it works now aligns better with the OpenColorIO design and the separation of concerns that is part of that.

Brecht, I'm not the enemy here. I'm speaking solely on the "authority" of understanding a basic level of colorimetry and having spent quite a few years around the OpenColorIO folks. I'm not making things up here.

The reason those legacy SPI sets exist, are not as demonstrations of best practices in OpenColorIO. They are specific configurations, based on legacy and such, that were shaped to work with OpenColorIO.

I've just had a longer thread in the OpenColorIO Slack reinforcing what I"ve just outlined here.

ACES does indeed include log transforms. Look for ACEScc, and ACEScct. Also remember, as pointed out by one of the Slack contributors, that OpenColorIO's intended design is limited and crippled by some of the choices existing DCC applications have chosen. The reason that ACES doesn't include looks, is that the actual RRT has a base "look" wound into it, with a specific protocol for Look Modification Transforms. That's not similar to Filmic's design, nor even OpenColorIO's basic design. Filmic is closer to an Arri LogC to Look approach. This latter approach was chosen specifically to support display referred applications more easily. While there is an ACES OpenColorIO configuration, the reference canonical implementation is not the OpenColorIO configuration, but rather the CTLs.

At any rate, I'll do up a Python configuration that will output Blender's OpenColorIO configuration. Can we outline what you'd like to see specifically in it, so I can make sure it works?

Color management discussions are confusing enough, I can't rely on an interpretation of a Slack conversation to make design changes. Nor do I see how what you relayed invalidates what I said. I wish the OpenColorIO folks would update the documentation to reflect the intended design.

The way I imagine it would be like this, ideally with all combinations of displays and view transforms possible.

  • Display: sRGB, Apple P3, ..
  • View Transform: Standard Transfer, Filmic, Filmic Log, ACES RRT, Raw, False Color
  • Look: None, Very Low Contrast, Low Contrast, Medium Low Contrast, Medium High Contrast, High Contrast, Very High Contrast

The looks could only be available for Standard Transfer, Filmic and ACES RRT. I'm not sure they make much sense for other view transforms, which would mostly be for analyzing the image.

Color management discussions are confusing enough, I can't rely on an interpretation of a Slack conversation to make design changes.

It's the best I have, apologies. The design I've loosely tried to describe is also a sane interpretation of how a Digital Content Creation application can deal with the confusing aspects of pixel management; concrete colorimetry based on Display is the most efficient method to keep the internal architecture as tidy as possible. Sadly, OpenColorIO was adopted into pipelines that had to negotiate their own legacy code, and isn't in the best position to adopt a "best practice" approach. The kludges you see everywhere are a byproduct of that.

Blender can do better here. I say that with 100% confidence, with a reasonable grasp of the complexities under the hood from both the Blender side, and a more general colorimetry consideration.

I wish the OpenColorIO folks would update the documentation to reflect the intended design.

The overhaul will likely happen at some point in the future, as V2 is further developed.

The way I imagine it would be like this, ideally with all combinations of displays and view transforms possible.

The issue here would be defining high value targets. See following:

Display: sRGB, Apple P3

Agree 100% given the number of creators living on Apple platforms, it makes sense to support their basic display colorimetry. I'll define sRGB and Display P3. That latter case sadly doesn't make it clearer for Apple folks, but I also suspect it keeps it out of the legal quagmire of using the label "Apple" in the configuration. Does this seem reasonable?

View Transform: Standard Transfer, Filmic, Filmic Log, ACES RRT, Raw, False Color

Can we consider dropping ACES RRT? The reasons I see are:

  1. The reference space isn't ACES AP0 nor ACES AP1, so this holds very little meaning.
  2. The RRT present up to 2.79 is a massaged hybrid based on the necessities of the ToS work. It bears no connection to the ACES RRT terminology, and is likely confusing. @Brecht Van Lommel (brecht) is well aware of this, and this information is provided for others involved in the thread; the RRT has no relationship to current ACES in any way.
  3. We can't offer a canonized ACES protocol driven approach, and any attempt we do on the Blender side will likely end up in a complete catastrophe trying to maintain synchronization with the AMPAS ACES development. Better to work Blender to be more pixel managed and permit arbitrary OpenColorIO configurations, including the official ACES 1.0.X branch?
  4. With high confidence we can infer it isn't widely used, and legacy support for it would be easily achieved via the existing Git repository.

Can we define what the purpose of Raw here is in terms of transforms? A non-colour transformed data dump, as-is, dumped directly to display? For example, if the blue primary intended a purple in terms of chromaticity, we'd just dump the code value, as data, of the intensity of the blue data channel as is?

Look: None, Very Low Contrast, Low Contrast, Medium Low Contrast, Medium High Contrast, High Contrast, Very High Contrast

Done. Given that Looks are index 0 based in OpenColorIO, I could in fact add None Look, which would end up with an index being created for the transform, even if it is essentially a no-op.

The looks could only be available for Standard Transfer, Filmic and ACES RRT. I'm not sure they make much sense for other view transforms, which would mostly be for analyzing the image.

This is a thing that frustrates me with OpenColorIO in that you can't attach Looks to particular View sets. I'm hoping this will be addressed when the metadata component of V2 is discussed. I've been bringing it up repeatedly. I'd love some help to make a case from the DCC side when the time comes. Software hard coding is a horrible idea obviously, but I don't see many options here. It stinks.

That would mean that the filtering would need to be addressed in software, based on this new configuration. Optionally, we could implement and use the Family facet to solve the filtering. This would aid pixel managing the UI as well.

How should we handle the customized roles?

  1. Given the VSE Cache reworking done by @Richard Antalik (ISS), and given that it enables proper respecting of the OpenColorIO reference space, should we do away with the sequencer role for the hacks around in the VSE?
  2. How should the default byte and float roles be handled? Would it be feasible to lean on the OpenColorIO filename support to lighten this as well as use the predefined roles checked in the OpenColorIO sanity checks? It would seem more ideal to use one of the mandatory roles here?

The way I imagine it would be like this, ideally with all combinations of displays and view transforms possible.

The issue here would be defining high value targets. See following:

Display: sRGB, Apple P3

Agree 100% given the number of creators living on Apple platforms, it makes sense to support their basic display colorimetry. I'll define sRGB and Display P3. That latter case sadly doesn't make it clearer for Apple folks, but I also suspect it keeps it out of the legal quagmire of using the label "Apple" in the configuration. Does this seem reasonable?

That's fine.

Can we consider dropping ACES RRT? The reasons I see are:

  1. The reference space isn't ACES AP0 nor ACES AP1, so this holds very little meaning.
  2. The RRT present up to 2.79 is a massaged hybrid based on the necessities of the ToS work. It bears no connection to the ACES RRT terminology, and is likely confusing. @Brecht Van Lommel (brecht) is well aware of this, and this information is provided for others involved in the thread; the RRT has no relationship to current ACES in any way.
  3. We can't offer a canonized ACES protocol driven approach, and any attempt we do on the Blender side will likely end up in a complete catastrophe trying to maintain synchronization with the AMPAS ACES development. Better to work Blender to be more pixel managed and permit arbitrary OpenColorIO configurations, including the official ACES 1.0.X branch?
  4. With high confidence we can infer it isn't widely used, and legacy support for it would be easily achieved via the existing Git repository.

I don't mind dropping the current RRT since it's so outdated. For compatibility with other apps I think it would be nice to have the latest RRT, it would work fine even if you don't get the benefits of wide gamut. But I wouldn't expect you to add it, and I'm not intending to spend time on this soon if at all. There are more important things to do regarding color management.

Can we define what the purpose of Raw here is in terms of transforms? A non-colour transformed data dump, as-is, dumped directly to display? For example, if the blue primary intended a purple in terms of chromaticity, we'd just dump the code value, as data, of the intensity of the blue data channel as is?

It would be a a non-color transformed data dump.

The looks could only be available for Standard Transfer, Filmic and ACES RRT. I'm not sure they make much sense for other view transforms, which would mostly be for analyzing the image.

This is a thing that frustrates me with OpenColorIO in that you can't attach Looks to particular View sets. I'm hoping this will be addressed when the metadata component of V2 is discussed. I've been bringing it up repeatedly. I'd love some help to make a case from the DCC side when the time comes. Software hard coding is a horrible idea obviously, but I don't see many options here. It stinks.

It would be good if OpenColorIO supported this, for now we can use a workaround in Blender. The look is named "Filmic - Very High Contrast" and then only appears for the "Filmic" view transform.

How should we handle the customized roles?

  1. Given the VSE Cache reworking done by @Richard Antalik (ISS), and given that it enables proper respecting of the OpenColorIO reference space, should we do away with the sequencer role for the hacks around in the VSE?
  2. How should the default byte and float roles be handled? Would it be feasible to lean on the OpenColorIO filename support to lighten this as well as use the predefined roles checked in the OpenColorIO sanity checks? It would seem more ideal to use one of the mandatory roles here?

We should not make any changes here for 2.80. There are a number of design changes that need to be made regarding color management, but I don't have time to define them exactly now. This needs to be done carefully.

We should not make any changes here for 2.80.

So am I to leave them in? I believe there are four baddies, which don’t make any sense:

  • rendering
  • default_byte
  • default_float
  • default_sequencer

rendering and default_sequencer are by far the most abusing roles I can see.

Can we keep this on topic? I realise there can be made bigger changes to color management in Blender, but for now this task was merely about making the current system less confusing by using clearer naming.

Or is changing names too big an issue with compatibility, meaning we should rather postpone naming changes until other changes are done to CM?

Can we keep this on topic?

It is on topic.

but for now this task was merely about making the current system less confusing by using clearer naming.

Does changing “Default” to “Standard” describe “clearer naming”?

Default is obviously not a good name for an option that is not the default :) so yes, that would be better I think.

But Standard is also not a very clear name. It’s best to be specific. Words like Default and Standard don’t really tell the user anything useful.

Words like Default and Standard don’t really tell the user anything useful.

Exactly on point.