AgX-Step1: Clean up no longer used colorspaces, display device, and LUTs #110559

Zijun Zhou · 2023-07-28T08:13:21+02:00

Zijun Zhou commented

2023-07-28 08:13:21 +02:00

This is the first step of AgX implementation as discussed in the original big AgX PR page

step 1. Clean Up. Delete all not longer used colorspaces and their LUT files.
This includes: nuke_rec709, lg10, XYZ display device and its standard view colorspace (there simply isn't a monitor you can buy on Amazon or Best Buy etc. that claims to be XYZ colorspace)

The None display device has duplicated functionality as the Raw view in sRGB display, will remove None device and rename Raw view to be None view instead.
(None view transform is also very straight forward, it's simply "No view transform applied")

As an extension of that, the Raw and Non-Color are duplicates of each other, we need to remove one of them. Since users are more familiar with Non-Color, and it's also the one assigned as the data role, will keep Non-Color and remove Raw.
Will change the None view (formerly Raw view) to use the Non-Color space. Will also add Raw as an alias of Non-Color

Ref #110685

This is the first step of AgX implementation as discussed [in the original big AgX PR page](https://projects.blender.org/blender/blender/pulls/106355#issuecomment-984699) >step 1. Clean Up. Delete all not longer used colorspaces and their LUT files. >This includes: nuke_rec709, lg10, XYZ display device and its standard view colorspace (there simply isn't a monitor you can buy on Amazon or Best Buy etc. that claims to be XYZ colorspace) > >The None display device has duplicated functionality as the Raw view in sRGB display, will remove None device and rename Raw view to be None view instead. >(None view transform is also very straight forward, it's simply "No view transform applied") > >As an extension of that, the Raw and Non-Color are duplicates of each other, we need to remove one of them. Since users are more familiar with Non-Color, and it's also the one assigned as the data role, will keep Non-Color and remove Raw. Will change the None view (formerly Raw view) to use the Non-Color space. Will also add Raw as an alias of Non-Color Ref #110685

Zijun Zhou added 9 commits 2023-07-28 08:13:33 +02:00

e8f6ddcbee Remove no longer used colorspaces and display device

The step 1 of AgX implementation

b438e61872 revert e8f6ddcbee

revert Remove no longer used colorspaces and display device

a99ab07213 Remove no longer used colorspaces and display device

Gitea did not show the proper "before & after" comparison the last time I committed this, so reverted that and trying to do that again, if it still doesn't, will leave it.

acafce0a96 Change `None` view's order to last, and update `active_displays` and `active_views`

ae1cab9283 Delete release/datafiles/colormanagement/luts/rec709.spi1d

1f64486e6f Delete release/datafiles/colormanagement/luts/lg10.spi1d

cc64da7459 Delete release/datafiles/colormanagement/luts/dci_xyz.spi1d

ada35c19b6 Delete release/datafiles/colormanagement/luts/vd16.spi1d

buildbot/vexp-code-patch-coordinator Build done. Details

02995ef46c Delete release/datafiles/colormanagement/luts/xyz_to_aces.spimtx

The config is already using OCIO's built-in transform for ACES, this LUT is not needed.

Zijun Zhou requested review from Brecht Van Lommel 2023-07-28 08:13:56 +02:00

Zijun Zhou requested review from Sergey Sharybin 2023-07-28 08:13:56 +02:00

Zijun Zhou referenced this pull request

2023-07-28 08:19:24 +02:00

Replace Default OCIO config with AgX (Filmic v2) #106355

Sergey Sharybin commented

2023-07-28 14:07:49 +02:00

@blender-bot build

Sergey Sharybin commented

2023-07-28 14:51:48 +02:00

Thanks for the patch!

I was testing the change locally and verified:

The XYZ/None displays are do-versioned to sRGB (so no invalid configuration on .blend file load)
The images which had Raw as an input color space are converted to Non-color color space
It is still possible to save EXR in XYZ space from the image save dialog.

There are couple of notes, which I think needs to be addressed.

One of them is the of the rename of Raw to None. The thing here is that statement "No view transform applied" has ambiguity: does it mean both display and view transforms are not performed, or is the display transform is still performed? Having it named as "Raw" kind of implies that no transforms performed at all.

Are you fine calling the view "Raw'? If so, do you mind updating the patch?

The other point was brought up by Brecht: there is still some code in Blender which does display name comparison to see whether color management is enabled or not. I think we should remove those name-based exceptions, and state that if no color management is desired, then the assets (image textures, i.e.) are to be set to non-color input color space. I think the code side would be easier to handle from our side, so I'll prepare patch separately.

Thanks for the patch! I was testing the change locally and verified: - The XYZ/None displays are do-versioned to sRGB (so no invalid configuration on .blend file load) - The images which had Raw as an input color space are converted to Non-color color space - It is still possible to save EXR in XYZ space from the image save dialog. There are couple of notes, which I think needs to be addressed. One of them is the of the rename of Raw to None. The thing here is that statement "No view transform applied" has ambiguity: does it mean both display and view transforms are not performed, or is the display transform is still performed? Having it named as "Raw" kind of implies that no transforms performed at all. Are you fine calling the view "Raw'? If so, do you mind updating the patch? The other point was brought up by Brecht: there is still some code in Blender which does display name comparison to see whether color management is enabled or not. I think we should remove those name-based exceptions, and state that if no color management is desired, then the assets (image textures, i.e.) are to be set to non-color input color space. I think the code side would be easier to handle from our side, so I'll prepare patch separately.

Zijun Zhou commented

2023-07-28 15:05:45 +02:00

does it mean both display and view transforms are not performed, or is the display transform is still performed? Having it named as "Raw" kind of implies that no transforms performed at all.

Interesting, but in OCIO context I don't think there is a "Display Transform" concept? As the Standard view transform is actually the "display transform", and it's still a view transform.

The thing about Raw is that it's another overly used term that can mean different things in different context, when Filmmakers say "Raw" they often refer to the bayer data from camera sensor (some of them have transfer function, like ARRI's RAW is in LogC AFAIK), in some color grading context people use "Raw" to mean it's "ungraded" which can be a log encoding.

I thought None is straight forward especically when the Look menu is default to None, when user want to turn off the view transform and do their own stuff in compositor, setting both view and look to None just sounds intuitive to me.

But if you confirm the decision to change it back to Raw, will do it.

> does it mean both display and view transforms are not performed, or is the display transform is still performed? Having it named as "Raw" kind of implies that no transforms performed at all. Interesting, but in OCIO context I don't think there is a "Display Transform" concept? As the `Standard` view transform is actually the "display transform", and it's still a view transform. The thing about `Raw` is that it's another overly used term that can mean different things in different context, when Filmmakers say "Raw" they often refer to the bayer data from camera sensor (some of them have transfer function, like ARRI's RAW is in LogC AFAIK), in some color grading context people use "Raw" to mean it's "ungraded" which can be a log encoding. I thought `None` is straight forward especically when the Look menu is default to `None`, when user want to turn off the view transform and do their own stuff in compositor, setting both view and look to `None` just sounds intuitive to me. But if you confirm the decision to change it back to `Raw`, will do it.

Sergey Sharybin commented

2023-07-28 15:51:27 +02:00

In "classical" OCIO the view and display transform indeed has a very strong coupling. However, on a user level it kind of makes sense to think of a view/look as an artistic choice, which is then presented on a specific display: https://docs.blender.org/manual/en/latest/render/color_management.html#display-transforms

I can see your points about the ambiguity of the term "Raw".
In a situation when both alternatives have cons and pros, I'd pick the one which is already in Blender. If you change the view back to "Raw" then I think we'll be able to land this patch (perhaps after #110580 and #110581, and also perhaps not right before going to a weekend -- so that we can be around to do any follow-ups if we missed something :)

In "classical" OCIO the view and display transform indeed has a very strong coupling. However, on a user level it kind of makes sense to think of a view/look as an artistic choice, which is then presented on a specific display: https://docs.blender.org/manual/en/latest/render/color_management.html#display-transforms I can see your points about the ambiguity of the term "Raw". In a situation when both alternatives have cons and pros, I'd pick the one which is already in Blender. If you change the view back to "Raw" then I think we'll be able to land this patch (perhaps after #110580 and #110581, and also perhaps not right before going to a weekend -- so that we can be around to do any follow-ups if we missed something :)

Zijun Zhou added 1 commit 2023-07-28 15:54:00 +02:00

7a8ff705ec Change `None` view back to `Raw` as requested

Zijun Zhou commented

2023-07-28 15:54:45 +02:00

Ok, None view is renamed back to Raw

Ok, `None` view is renamed back to `Raw`

Zijun Zhou added 1 commit 2023-07-28 16:01:43 +02:00

621f223971 Update active_views

Sergey Sharybin approved these changes 2023-07-28 17:14:16 +02:00

Sergey Sharybin left a comment

Thanks for the update. There is small inlined question, just to clarify. Don't consider it is a stopper.

release/datafiles/colormanagement/config.ocio Outdated

						
				@ -169,9 +119,10 @@ colorspaces:

				  - !<ColorSpace>

				    name: Non-Color

				    aliases: [Generic Data, Non-colour Data, Raw, Utility - Raw]

Sergey Sharybin commented

2023-07-28 17:11:26 +02:00

In Blender we follow American English, so it is Color. But I guess the purpose of the "Non-colour Data" is some sort of compatibility with other configurations?

Zijun Zhou commented

2023-07-28 17:19:47 +02:00

Yes it's a compatiibility with the original Filmic-Blender on GitHub. Though upon double check, they used capital letter for "Colour". will update the fix.

Zijun Zhou added 1 commit 2023-07-28 17:23:03 +02:00

7336bd33b3 Use capital `Non-Colour Data` after double checking Filmic-Blender on GitHub

Sergey Sharybin merged commit b2b7b37139 into main

2023-08-01 14:57:53 +02:00

Sergey Sharybin deleted branch AgX-Step1

2023-08-01 14:57:55 +02:00

Sergey Sharybin commented

2023-08-01 14:59:54 +02:00

Merged to the main now. I did add some details and background on the options which are removed, to more explicitly show what their purpose was. Hope you don't mind.

I also did couple of other changes needed on the C side to improve compatibility of currently saved .blend files.

Merged to the main now. I did add some details and background on the options which are removed, to more explicitly show what their purpose was. Hope you don't mind. I also did couple of other changes needed on the C side to improve compatibility of currently saved .blend files.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

AgX-Step1: Clean up no longer used colorspaces, display device, and LUTs #110559