Materials incorrect color space using AGX

[xmf]

AGX is a new color management developed by Troy Sobotka the maker of Filmic color management, AGX is vastly superior in every regard however it is not yet officially integrated in blender and requires replacement of color management files in blender. I think it would be worth adding support for it because more and more people are using it and I think it should be trivial to add support for it so that the materials that should be imported as “Non-Color” are properly imported as “Generic Data” in AGX and not sRGB which then requires manual adjustment for what can sometimes be a lot of image nodes.

This is the official thread where the ongoing AGX development is being discussed:

https://blenderartists.org/t/feedback-development-filmic-baby-step-to-a-v2/1361663

This is the lates up-to-date AGX repository that should be used, it is not official but Troy is working on it and everything will be merged into an official repository at some point in future:

https://github.com/EaryChow/AgX

This is the original AGX repository for reference only, should not be used for anything at the moment as it is terribly outdated:

https://github.com/sobotka/AgX

[Alessandro Padovani]

srgb and non-color refer to the texture space, not to the output.

[xmf]

not sure what you mean, but to make it more clear diffeomorphic daz importer imports textures that should be non-color as srgb when you use agx. so a bump or specular map will be set to srgb because non-color doesnt exist in agx. so again, a patch that detetcts agx and properly assigns those non-color textures to their counterpart in agx which is “Generic Data” - would be nice.

[xmf]

adding “Generic Data” to the input list at line 823 of material.py seems to fix the issue

[3 Pood]

It’s interesting because an Alias is set for that for both Non-Color and Raw so it’s possible that is an issue with the bpy interface (because Blender should be supporting OCIO 2 aliases). Then again, one might also try setting is_data to True rather than setting the name property for non-color textures

[Alessandro Padovani]

The filmic transform defines a tone mapping function from the linear space to the output space so it doesn’t affect the texture color spaces in any way. From your description I supposed agx to be a “filmic transform v2“ so something similar with just a different transfer function.

Sorry I don’t have time to read the entire agx documentation and I don't use it myself, so please describe what’s the issue here and the solution to help Thomas. You say that’s “trivial“ so this should be easy. Anyway since agx is not included in the official blender distribution, it may be a waste of time to support it in beta stage.

If agx defines new names for the existing color spaces I guess those could be included, though in this case aliases should do fine as @3Pood noted, so everything should work already. That is if you added the OCIO profiles correctly. Did you ?

[3 Pood]

so please describe what’s the issue here and the solution to help Thomas

The issue is that the method used to generate texture nodes hard codes names to attempt and if it fails uses the default. In this case, "Non-Color", "Linear", "Non-Colour Data" is insufficient. Also “Linear” is not the same thing as “Non-Color” as there are multiple linear spaces based on primaries and white point. Non-color says literally “do not try to change these values”. Daz is not color-managed though, so it’s not as important. The only time when linear would be used in Daz is HDRIs, and would be Linear with BT.706 primaries.

It’s hard to know exactly where the true issue is:

1. It’s Blender’s bpy API’s fault, because it does not handle aliases well, so when you set a name that is correctly aliased in the OCIO config, it is not treated correctly. It DOES when opening files: if “Non-Color” were set in a saved Material it would map correctly. So it does seem likely this was an oversight on their part
2. Diffeomorphic is setting non-color incorrectly, instead it should be setting the is_data flag to True. I haven’t tested this, might work but might not
3. Diffeo should just add all of the aliases as they come up for configs like ACES or other OCIO profiles that people use

From my perspective, 1 is the correct thing but even if fixed if compatibility is important it’s still a bug. Changing the OCIO color profile is a standard feature already, AgX is just a different profile in use, it’s not necessarily going to be shipped with Blender like Filmic was. Blender just supports alternate OCIO profiles.

If 2 works, this might be the best option. Since this is converting out of Daz (which is not color managed), assuming sRGB for color, and Linear BT.706 for HDRI maps is valid. If setting is_data works correctly (I cannot test right now), then this is the simplest fix that will work with any OCIO config. If not, then adding “Generic Data” and whatever other data aliases come up is probably going to be the only backward-compatible method.

‌

[xmf]

@Alessandro Padovani Yeah its installed correctly. I don’t understand what is it you don’t understand, can you elaborate a bit more please?

Step 1: I install AGX into blender

Step 2: I import a duf/dbz export from daz, an object with some materials in it.

Result: imported object’s textures that should have had their color space assigned to “non color” are assigned to sRGB, which is incorrect and requires manual fixing.

Solution: “adding “Generic Data” to the input list at line 823 of material.py seems to fix the issue” but maybe there is some other solution? I - don’t know.

[Alessandro Padovani]

@xmf I don’t use AGX, you have to explain for @Thomas please wait for his answer.

[3 Pood]

@Alessandro Padovani It’s not really a specific issue of AgX, it’s an issue with OCIO 2.0 profiles in general. For example, ACES profiles would run into this as well since the ACES config defines no data-texture transform that matches the search list here, and aliases are not respected with the image bpy bindings (and probably others as well, like video sequencer).

Is it a diffeomorphic plugin issue specifically? No. Can diffeo work around it? Yes. Should Blender still fix it? Absolutely.

[Alessandro Padovani]

@Thomas please let us know if you understand this or need more information so @3Pood can help. Thank you.

[xmf]

@3 Pood I havent noticed this problem when loading blend files saved with non-color data, it correctly loads them as generic data. I’ve only noticed the issue in diffeomorphic import context

[3 Pood]

@xmf I said that in my second post. The issue is with the bpy API. Blender itself supports OCIO aliases as of this commit.

It’s just the python API that does not recognize aliases as valid when you try to set the color_space name property.

[Thomas Larsson]

I cannot say that I understand this stuff, but it easy enough to add "Generic Data" to the list of non-srgb types, so I did that.

[Alessandro Padovani]

Please @3Pood let us know if you have anything to add so we can mark as resolved.

[3 Pood]

Probably need to add ‘raw’ and ‘Utility - Raw’ for ACES support as those are data roles (the former is defined as an alias but the next version will rely on OCIO 2.0 aliases and be removed, so need the explicit definition). And remove Linear: that should only be used for actual Linear BT.706 data, as it will cause issues if the scene is rendered in BT.2020 or ACEScg linear. In DAZ it’s only going to show up in HDR environment textures.

So, in my opinion, the line should be:

setColorSpace(img, ["Non-Color", "Raw", "Non-Colour Data", "Generic Data", "Utilities - Raw"])

You might consider adding srgb_texture to the list above it as well, although it will default to whatever is aliased for sRGB anyway so it might be pointless.

Of course, this will only work for the selected color configs, but they are going to be the most commonly used. The best case is Blender respecting aliases in the bpy API.

[Thomas Larsson]

Change added in last commit.

[Alessandro Padovani]

@Thomas, I see in commit d05b341 you didn’t add “Raw“ and didn’t remove “Linear“ as suggested by @3Pood. As for “Linear” we import the HDRIs from daz that are in linear space so this should be kept in my opinion, I have no clue what kind of issues it may cause with BT.2020 please @3Pood can you elaborate ?

I also understand that OCIO is not fully supported by blender yet ? If so would it be better to avoid BT.2020 and use BT.709 instead until blender fixes it ?

https://developer.blender.org/T68926

https://devtalk.blender.org/t/filmic-as-default-is-hurting-the-experience-for-people/22283/211

[3 Pood]

As for “Linear” we import the HDRIs from daz that are in linear space so this should be kept in my opinion, I have no clue what kind of issues it may cause with BT.2020 please @3Pood can you elaborate ?

The idea of “Raw” and “Non-color” and other “data” roles, is that no transform is applied to them. This is true regardless of what they are specced for (ACES has like 4 data roles) and is the purpose of the isdata OCIO color space attribute.

Linear is actually a color space, in this case, by default “Rec.709 Linear”. This also happens to be the default scene_linear in Cycles/Eevee rendering so no transform is applied. But when you change the scene linear space, then a transform needs applied. The danger here is twofold: if a data texture is marked Linear, it will be transformed into the scene linear space leading to incorrect data. And if a Linear BT.709 file is file is marked as data, it will NOT be transformed, leading to artifacts.

So, the ideal would be to special case the environment texture and set it explicitly to Linear or Linear Rec.709 (ACES defines “Utilities - Linear - Rec.709” with “lin_rec709” as an alias)

You could add another colorspace flag like LINEAR and in render.py in buildEnvmap pass that instead of NONE

I also understand that OCIO is not fully supported by blender yet ? If so would it be better to avoid BT.2020 and use BT.709 instead until blender fixes it ?

Blender renders fine in general in alternate color spaces, the issue is mostly around the UI (notoriously the color picker) and some of the procedural nodes (which half work, as long as you don’t pick a white point outside of D65). People can and do render with alternate color spaces including ACES in Blender.

But regardless, setting Data (raw, non-color, etc) to Linear and linear textures to data is wrong, and will have side effects.

[Alessandro Padovani]

@3Pood From your explanation I understand one thing that I agree, the environment texture should be set as “Linear“, while actually it’s “Non-Color“. I don’t understand the “twofold danger” unless it’s a blender bug, but I trust you. Would it be correct for @Thomas to do the following changes ?

1. Add “Raw“ and remove “Linear“ from setColorSpace()
2. set the environment texture as “Linear“

‌

[3 Pood]

From my understanding, these are the changes needed:

In material.py line 819

        if img:
            if colorSpace == "COLOR":
                setColorSpace(img, ["sRGB", "sRGB OETF", "srgb_texture"])
            elif colorSpace == "NONE":
                setColorSpace(img, ["Non-Color", "Raw", "Non-Colour Data", "Generic Data", "Utilities - Raw"])
            elif colorSpace == "LINEAR":
                setColorSpace(img, ["Linear", "Linear BT.709 I-D65", "Linear BT.709", "Utilities - Linear - Rec.709", "lin_rec709")

In render.py line 231

        img = self.getImage(envmap, "LINEAR")
        tex = None
        if img:
            tex = self.addNode("ShaderNodeTexEnvironment", 3)
            self.setColorSpace(tex, "LINEAR")

And that should be enough to cover most cases.

[3 Pood]

I don’t understand the “twofold danger” unless it’s a blender bug

The twofold danger I talk about is with the current implementation it is possible to tag an image texture node incorrectly in two ways: one, a data texture can be tagged “Linear” if “Non-Color” is not a name defined in the config but “Linear” is. And second, a Linear BT.709 I-D65 texture can be marked “Non-Color” as it’s defined first.

‌

In either case it won’t cause an issue if the render space exactly matches Linear BT.706 I-D65, which is default, but anything not default (even if it still used BT.706 primaries but used I-E white point) would get incorrect results because either the wrong transform or no transform was applied respectively.

[Alessandro Padovani]

Ok thank you please @Thomas let us know if it’s clear to you to proceed.

[Thomas Larsson]

Changes made in last commit. The call to setColorSpace in render.py was redundant; it was a left-over from Blender 2.79 when the color space was defined in the texture node rather than in the image itself. There was another function in cycles.py with the same name that was also removed.

[Alessandro Padovani]

Commit 6637ac9 works fine here also the code looks right, but I can’t test alternative color spaces for the output since I have a srgb monitor. If @3Pood has nothing to add we may close as resolved.

[3 Pood]

but I can’t test alternative color spaces for the output since I have a srgb monitor

That’s not really required, display transforms are separate from render space. Your device transform is still sRGB regardless of the color config. It’s about how the color data is processed

Anyway, tested with AgX configs. Color vs data was imported correctly, and the environment map set as Linear BT.709 I-D65 as expected. I say it’s good to close out.

[Alessandro Padovani]

@3Pood Yes I understand that ACES is a replacement for the default color spaces, with different tone mapping and better pipeline integration. Personally I don't use it because I don't need a pipeline integration and I believe tone mapping is an "artistic choice". I don't even like too much the default "filmics" and tend to use “standard” for better rim lights and on purpose over exposition.

Thank you 3Pood and Thomas for this nice fix to support OCIO alternative color spaces.