Page MenuHome

Python API: add mathutils.Color functions to convert color spaces
ClosedPublic

Authored by Brecht Van Lommel (brecht) on May 19 2022, 9:25 PM.

Details

Summary

Between scene linear and sRGB, XYZ, linear Rec.709 and ACES2065-1.

And add some clarifications about color spaces in the docs.

Fixes T98267

Ref T68926

Diff Detail

Repository
rB Blender
Branch
color-attr (branched from master)
Build Status
Buildable 22183
Build 22183: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) requested review of this revision.May 19 2022, 9:25 PM
Brecht Van Lommel (brecht) created this revision.

I'll probably add to_linear_rec709 and to_aces as well, if this API structure makes sense.

We could also add general OpenColorIO colorspace conversions to the API at some later point, but that would be solving a different problem. For importers and exporters it's important to be able to know what the colorspaces is in an absolute sense, whereas color space names in OpenColorIO configs can be arbitrary.

While most colors (by default are in linear color space), this API is returning colors in other spaces, meaning it's possible to have mathutils.Color in other color-spaces.

The down side of this is any further operations on them might not work as expected.

Would it be better to have mathutils.Color to store a space? This would simplify conversion functions since you could call to_xyz(...) knowing that whatever color-space the color is in, it will use the correct conversion, removing the need for from_* & the assumption that all colors are linear.

I am not really convinced making Color aware of its space is something what really makes life easier. For the colors coming from RNA we know the color space, but Color object might be created by a script. This means that either we accept Color without a space attached to it anyway (so that we do not break API) or we will introduce a huge breaking change and introduce an entire complication of creating colors.

I also believe it is not a new "problem" that there is no space attached to the Color: IO scripts already need to take care of conversion from either scene linear or sRGB when dealing with matertials/legacy vertex colors.

Moving the burden of implementing color space conversion logic from scripts to BPY is something we indeed need to do. The proposed patch mainly seems good to me. It does clarify a lot of things docstrings. The only thing I'd do different is naming to solve the ambiguity: to_xyz -> linear_to_xyz (or scene_linear_to_xyz if we do not want to have linear imply scene linear). This will help catching semantic issues from just seeing or typing the code (avoiding thought process indirection). I believe it will also solve the concern of Campbell.

(or scene_linear_to_xyz if we do not want to have linear imply scene linear). This will help catching semantic issues from just seeing or typing the code (avoiding thought process indirection).

Yes explicitly stating ‘scene linear’ is a good idea especially since CIE XYZ D65 is the display referred exchange color space.

If having the color space stored in the color complicates things to much, having more explicit functions seem reasonable, I find the to/from in this patch a bit confusing though.

These are straightforward, at the expense of being more verbose.

  • .to_srgb_from_linear()
  • .to_srgb_from_xyz()
  • `.to_linear_from_srgb()' ... etc.

Another option could be to have the from as an argument, defaulting to linear.

  • .to_srgb(from='XYZ') for e.g, that has the advantage that if we support more color spaces in the API the number of methods wont increase so much, although in this case I doubt that would be a problem.
  • Add scene linear in the name
  • Add linear Rec.709 and ACES
Brecht Van Lommel (brecht) retitled this revision from Python API: add utility functions to convert to/from sRGB and XYZ to Python API: add mathutils.Color functions to convert color spaces.May 20 2022, 2:34 PM
Brecht Van Lommel (brecht) edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.May 20 2022, 4:26 PM

Just for the records, this works although it looses a bit of precision.

For instance: if the byte color is 179, it becomes 178 after the conversion. I'm not sure if it is relevant.

[EDIT: it turns out to be 178.9 so it is fine]

Color(C.object.data.attributes.active_color.data[0].color[:3]).scene_linear_to_srgb()[0] * 255
178.97117167711258

intern/opencolorio/fallback_impl.cc
246–247

Without specifying the LINEAR component, it appears that this refers to the HDTV color space.
This goes for the other occurrences too.

intern/opencolorio/ocio_capi.h
40

Needs the AP0 to avoid confusion with the AP1 ACEScg color space.
This goes for the other occurrences too.

Having all these methods with color space names as top-level methods clutters the namespace. Using a single prefix that makes it clear these functions are conversions and groups them more usefully:

With this patch auto-completion shows:

>>> color = Color()
>>> color.
          aces_to_scene_linear(
          b
          copy(
          freeze(
          g
          h
          hsv
          is_frozen
          is_valid
          is_wrapped
          owner
          r
          rec709_linear_to_scene_linear(
          s
          scene_linear_to_aces(
          scene_linear_to_rec709_linear(
          scene_linear_to_srgb(
          scene_linear_to_xyz_d65(
          srgb_to_scene_linear(
          v
          xyz_d65_to_scene_linear(

Using to/from terms would group them more usefully, eg:

>>> color.
-- snip ---
          to_aces_from_scene_linear(
          to_rec709_linear_from_scene_linear(
          to_scene_linear_from_aces(
          to_scene_linear_from_rec709_linear(
          to_scene_linear_from_srgb(
          to_scene_linear_from_xyz_d65(
          to_srgb_from_scene_linear(
          to_xyz_d65_from_scene_linear(

Other names could work too but .convert_* for e.g. but end up being more verbose.

If we are to use from/to can we use from *before* to?

From/to ordering would be fine by me (although no strong opinion either way).

I find the "reversed" order of "from" and "to" component confusing and harder to follow than a more natural semantic "from FOO to BAR". If we are to name functions in a way that visually groups them then I'd say lets use "from_" prefix. For example from_scene_linear_to_xyz.

Out of curiosity: the new functions all do two conversions, via XYZ; is there no need to expose that as IMB_IMB_colormanagement_…_to_xyz and IMB_colormanagement_xyz_to_… functions?

Another thing, which is very minor and feel free to completely ignore. The two matrix multiplications in each function feel a bit weird to me. From a clarity of code perspective, and from the single responsibility principle, I would expect each conversion to be its own function, and then RGB → REC709 would do two function calls for RGB → XYZ and XYZ → REC709. I can understand the need to save some function calls and get these functions more performant, but in that case, wouldn't it be better to also halve the number of matrix multiplications, by precomputing the RGB → REC709 matrix and using that directly?

About the naming: I agree that from_…_to_… is clearer.

Ok, will rename to from_scene_linear_to_xyz.

Out of curiosity: the new functions all do two conversions, via XYZ; is there no need to expose that in IMB_IMB_colormanagement_…_to_xyz and IMB_colormanagement_xyz_to_… as functions?

Another thing, which is very minor and feel free to completely ignore. The two matrix multiplications in each function feel a bit weird to me. From a clarity of code perspective, and from the single responsibility principle, I would expect each conversion to be its own function, and then RGB → REC709 would do two function calls for RGB → XYZ and XYZ → REC709. I can understand the need to save some function calls and get these functions more performant, but in that case, wouldn't it be better to also halve the number of matrix multiplications, by precomputing the RGB → REC709 matrix and using that directly?

The fact that XYZ is used as an intermediate space internally is an implementation detail, which might change at some point. Conceptually we should be thinking of scene linear as the main color space that you can convert to/from for file I/O, or convert from for display.

I'm going to do cleanup commit after this with some renaming for clarity, I can also precompute the matrices then to avoid having to do two matrix multiplications.

intern/opencolorio/fallback_impl.cc
246–247

The fact that it's a matrix transform to me implies it's linear.

intern/opencolorio/ocio_capi.h
40

The comment clarifies that. The proper name as suggested by the standard really is "ACES2065-1", but I think using "ACES" and "ACEScg" in API names is enough to distinguish them.

Add from_ prefix to names.

Campbell Barton (campbellbarton) added inline comments.
source/blender/python/mathutils/mathutils_Color.c
1018

Looks like a typo?

Fix missing "to" in one of the names