I ran the MRP and I noticed that you have the material set to treat vertex colors as linear (i.e. vertex_color_is_srgb is false). Is that intentional?

For clarity, when vertex_color_is_srgb is false you are telling the renderer to treat the vertex color as a linear color. However, in the compatibility renderer vertex colors are always sRGB.

I confirmed that enabling vertex_color_is_srgb makes the colors match in both renderers.

I think we should likely add an sRGB->linear conversion for when vertex_color_is_srgb is false to avoid this sort of issue. But we need to think about that more in terms of how it impacts asset pipeline etc.

For context, the setting currently does the following:

if (flags[FLAG_SRGB_VERTEX_COLOR]) {
	code += "	if (!OUTPUT_IS_SRGB) {\n";
	code += "		COLOR.rgb = mix(pow((COLOR.rgb + vec3(0.055)) * (1.0 / (1.0 + 0.055)), vec3(2.4)), COLOR.rgb * (1.0 / 12.92), lessThan(COLOR.rgb, vec3(0.04045)));\n";
	code += "	}\n";
}

OUTPUT_IS_SRGB is true in the compatibility renderer. So when using the compatibility renderer this code never runs.

We could add an else branch that essentially does the opposite. i.e.:

if (flags[FLAG_SRGB_VERTEX_COLOR]) {
} else {
	code += "	if (OUTPUT_IS_SRGB) {\n";
	code += "		// convert to linear
	code += "	}\n";
}

I ran the MRP and I noticed that you have the material set to treat vertex colors as linear (i.e. vertex_color_is_srgb is false). Is that intentional?

No clue, is there some way to change that in Blender? I think it's set to byte colors, which sounds like sRGB, but I'm not very familiar with creating 3D models, material settings, and how that makes it into Godot.

The color attribute data type in Blender (Color/Byte Color) does not affect the colorspace stored in the .glb file, which is always Linear.

I also am seeing this issue in Godot v4.3.dev6.mono

not sure if its godot or the gltf exporter in blender. i dont think its a srgb thing because if i pass the COLOR.rgb through the formula clayjohn posted.. its gets even darker, almost black. here is blender:

and in godot:

Windows 10.0.19044 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (NVIDIA; 31.0.15.5161) - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

@bertodelrio256 Are you sure this isn't due to fog or ambient lighting appearing on the materials, making them look washed out? The green coloring seems to look correct.

@bertodelrio256 Are you sure this isn't due to fog or ambient lighting appearing on the materials, making them look washed out? The green coloring seems to look correct.

i dont think so.. I have both ambient and reflected light disabled on the worldenvironment I do have fog but it looks the same when disabled and this is in my shader:

shader_type spatial;
render_mode ambient_light_disabled, specular_disabled, shadows_disabled;
#include "DSfuncs.gdshaderinc"

group_uniforms uvs;
uniform vec2 uv_scale = vec2(1);
uniform vec2 uv_shift = vec2(0);
uniform vec2 uv_scroll = vec2(0);

group_uniforms textures;
uniform bool dynamic_object = false;
uniform sampler2D color_texture : source_color;
uniform float highlight_boost = 0.0;
uniform float highlight_contrast = 1.0;
uniform bool use_cubemap = false;
uniform bool flip_cubemap = false;
uniform samplerCube cubemap : source_color;
uniform float cubemap_power = 4.0;

group_uniforms emission;
uniform bool use_emission = false;
uniform bool use_emission_mask = false;
uniform sampler2D emission_texture : source_color; //alpha is mask
uniform vec4 emission_tint : source_color = vec4(1);
uniform float emission_power = 0.0;
uniform vec2 emmask_uv_scale = vec2(1);
uniform vec2 emmask_uv_shift = vec2(0);
uniform vec2 emmask_uv_scroll = vec2(0);

varying vec2 uvs;
varying vec2 emmask_uvs;
varying float metal;
varying float wet;
varying vec4 vertcol;
global uniform sampler2D ambient_gradient : source_color, repeat_disable;


void vertex()
{
    uvs = (UV * uv_scale + uv_shift) + ((TIME * 0.01) * uv_scroll);
	if(use_emission_mask)
	{
		emmask_uvs = (UV * emmask_uv_scale + emmask_uv_shift) + ((TIME * 0.01) * emmask_uv_scroll);
	}

	vertcol = COLOR;
}

void fragment()
{
    ALBEDO = texture(color_texture, uvs).rgb;
	ALBEDO += ALBEDO * (pow(ALBEDO.r + ALBEDO.g + ALBEDO.b, highlight_contrast) * highlight_boost);


    if(use_cubemap)
    {
		vec3 view_vec;
		if(flip_cubemap)
		{
			view_vec = VIEW;
		}
		else
		{
			view_vec = -VIEW;
		}
		vec3 ref_vec = reflect(view_vec, NORMAL);
		ref_vec = (INV_VIEW_MATRIX * vec4(ref_vec, 0.0)).xyz;
		vec3 ref_col = texture(cubemap, ref_vec).rgb;

		ALBEDO *= ref_col * cubemap_power;
    }

	if(use_emission)
	{
		vec3 em_temp = vec3(0);
		em_temp = texture(emission_texture, uvs).rgb;
		if(use_emission_mask)
		{
			em_temp *= texture(emission_texture, emmask_uvs).a;
		}
		EMISSION += em_temp * emission_tint.rgb * emission_power;
	}

	ALBEDO *= 4.0;
}

void light()
{
	if(dynamic_object)
	{

	}
	else
	{
		DIFFUSE_LIGHT += vertcol.rgb;
	}
}

the green lighting is correct, is the dark areas that are wrong.

@bertodelrio256 Please upload a minimal reproduction project to make this easier to troubleshoot.

Also, the issue you're encountering here is unrelated as you're using Forward+, not Compatibility.

PS: Code blocks should use triple backticks like this (with an optional language name for syntax highlighting):

```glsl
code here
```

I edited your post accordingly, but remember to do this in the future 🙂

will do

opened new issue here: #92404

thanks!

I just came across this issue today and I'm confused by the proposed solution @clayjohn. As mentioned previously it seems glTF stores vertex colours as linear (see KhronosGroup/glTF#1638). So, unless I'm misunderstanding, isn't the solution to this issue to have OUTPUT_IS_SRGB not be true by default in the Compatibility renderer and instead allow the same functionality of setting vertex_color_is_srgb that we have in Forward+ and Mobile? My proposed solution now in comment below

EDIT: I decided to also do the simplest possible test I could think of to double check this. So I made a plane in Blender with all vertex colours set to 50% grey. Imported it into Godot and switched between renderers and setting vertex_color_is_srgb to produce the following images. Note these are all being displayed completely unshaded. This seems to confirm that Forward+ (and also Mobile) can correctly interpret glTF vertex colours when vertex_color_is_srgb is set to false as they are stored as linear colours but Compatibility cannot because that option doesn't exist. As expected, Compatibility gives the same incorrect output as Forward+ with vertex_color_is_srgb set to true.

The results above also appear to agree with @caimantilla's original issue that Compatibility incorrectly displays vertex colours as darker than they should be.

Ok, I think I finally understand this and what needs to happen to fix it.

What happens currently
glTF stores vertex colors as linear. Therefore when using Forward+/Mobile and having vertex_color_is_srgb set to false the existing conversion code in material.cpp (which converts sRGB into linear) doesn't run and everything looks as expected.

If you enable vertex_color_is_srgb when using Forward+/Mobile the conversion happens (as OUTPUT_IS_SRGB is false for those renderers) but in the case of glTF it's converting an already linear colour into linear again which makes it much darker. Obviously if the file does have vertex colors encoded as sRGB because it's a different format or an off-spec glTF this option is useful.

In Compatibility OUTPUT_IS_SRGB is true and so vertex_color_is_srgb currently has no effect. Therefore the linear colours in the glTF are assumed to be sRGB and used as is. Again, this makes them much darker than intended, identical to having vertex_color_is_srgb enabled in Forward+/Mobile.

Proposed fix
Broadly as @clayjohn suggested I think there should be an else statement so that if vertex_color_is_srgb is false something different happens. However we only want something to happen if we're using Compatibility (as Forward+/Mobile currently work as expected) so, yes, the subsequent code should only run if OUTPUT_IS_SRGB is true. However (and I don't know whether this was simply a typo) it needs to be a Linear -> sRGB conversion, not the other way around. This is what confused me yesterday. This then means that a glTF with linear vertex colours will have them converted into sRGB before they're used by the Compatibility renderer.

In the case of a file format or off-spec glTF that has its vertex colours encoded as sRGB being used in Compatibility this will work fine with vertex_color_is_srgb set to true as no conversion will occur because OUTPUT_IS_SRGB is also true. Also if any existing users wanted to maintain the darker, incorrect of version of the vertex colours from a glTF they would just need to set vertex_color_is_srgb to true as well. I don't know whether this is considered a breaking change as the colours are currently just incorrect. Apart from this I don't currently foresee any other negative effects from this change.

Summarised in pseudo-code:

if vertex_color_is_srgb
	if !OUTPUT_IS_SRGB
		sRGB -> linear conversion
else
	if OUTPUT_IS_SRGB
		linear -> sRGB conversion

I don't know whether anyone wants to check my logic on this? @Calinou?

I'm probably out of my depth actually implementing this but I could read the docs and give it a go if that would be useful?

I don't know whether anyone wants to check my logic on this? @Calinou?

I don't know for sure if it's the correct fix. I suggest trying to take screenshots in Forward+ and Compatibility with this change and see how it looks in practice.

Here's the shader code that is toggled by enabling Vertex Color is sRGB:

You should be able to modify it to perform an inverse conversion depending on OUTPUT_IS_SRGB with an if statement. This is less optimal than using the shader preprocessor, but the shader preprocessor is currently not working in BaseMaterial3D due to a bug. If the shader preprocessor worked, you could do the following so that only the relevant branch is compiled into the shader:

#if CURRENT_RENDERER == RENDERER_COMPATIBILITY
// Compatibility only
#else
// Forward+/Mobile only
#endif

This way, you avoid a runtime if conditional and improve performance slightly.

Thanks @Calinou

So to test this I've created a shader with the following code in the vertex function:

if (OUTPUT_IS_SRGB) {
		COLOR.rgb = mix(
				(pow(COLOR.rgb, vec3(1.0 / 2.4)) * (1.0 + 0.055)) - vec3(0.055),
				COLOR.rgb * 12.92,
				lessThan(COLOR.rgb, vec3(0.0031308)));
}

I got the linear to sRGB conversion formula from https://entropymine.com/imageworsener/srgbformula/ and I've tried to match the style of the existing sRGB to linear conversion code. Interestingly that source uses ≤ rather than < but I don't see how that would make any significant real world difference so I've used lessThan to match the existing code.

The good news is that this seems to work. With this shader code running on my existing 50% grey plane I get the following results. Godot is now displaying the colour consistently between renderers!

As an aside it does appear that Godot is displaying the colour very slightly differently compared to Blender (R is 127, rather than 128) but such a tiny difference is obviously imperceptible. However in this case the difference might actually be being introduced by Blender's glTF encoding as I get the exact same colour as Godot if I import the exported glTF back into Blender. I'm assuming the difference is just due to conversions not being perfectly accurate or rounding differences. Regardless, it has nothing to do with my shader code as it's the same in Forward+ where my code doesn't run.

I also tried a new, random colour. Godot continues to display the colour consistently between renderers.

Again there's the imperceptible difference in colour compared to Blender (though this time importing the glTF back into Blender stays consistent with the original model 😕)

So, in conclusion, it would appear that for any users who want an interim fix to this issue they just need to use a shader that incorporates the code at the start of this comment in the vertex function. It's also worth noting that if (OUTPUT_IS_SRGB) isn't necessary if you're definitely sticking with Compatibility for a project and aren't going to switch between renderers.

If I'm understanding correctly, as it stands now the relevant part of material.cpp would need to look like this to incorporate this fix but should we be waiting until the shader preprocessor is fixed?

	if (flags[FLAG_SRGB_VERTEX_COLOR]) {
		code += R"(
	// Vertex Color is sRGB: Enabled
	if (!OUTPUT_IS_SRGB) {
		COLOR.rgb = mix(
				pow((COLOR.rgb + vec3(0.055)) * (1.0 / (1.0 + 0.055)), vec3(2.4)),
				COLOR.rgb * (1.0 / 12.92),
				lessThan(COLOR.rgb, vec3(0.04045)));
	}
)";
	} else {
		code += R"(
	if (OUTPUT_IS_SRGB) {
		COLOR.rgb = mix(
				(pow(COLOR.rgb, vec3(1.0 / 2.4)) * (1.0 + 0.055)) - vec3(0.055),
				COLOR.rgb * 12.92,
				lessThan(COLOR.rgb, vec3(0.0031308)));
	}
)";
	}

If I'm understanding correctly, as it stands now the relevant part of material.cpp would need to look like this to incorporate this fix but should we be waiting until the shader preprocessor is fixed?

That looks correct. I'm not sure when the shader preprocessor will be fixed in BaseMaterial3D, so I think we'll keep to have using if especially if we want this to be cherry-picked to 4.4.x.

cc @clayjohn