wine icon indicating copy to clipboard operation
wine copied to clipboard

fshack: The gamma ramp data doesn't query the actual array layout which can lead to incorrect rendering

Open gerddie opened this issue 2 years ago • 3 comments

Debugging an issue running Tomb Raider II by using the mesa/virgl driver within a Qemu VM showed a problem with the gamma correction as done with fshack: The shader doesn't specify any layout for the gamma array values which results that in this case the array values being allocated with a stride of 16 byte (vec4), but when the data is uploaded by using glBufferData it expects that the array is densely packed, and the result is that the "gamma corrected" output has only an incorrect red component.

According to OpenGL 4.6 (Core profile) May 14, 2018, section 7.6.2.2. the uniforms contained within a uniform block are extracted from buffer storage in an implementation-dependent manner and the offsets and alignments can by queried by using glGetActiveUniformsiv, and this latter step is missing here.

To avoid the querying and the dynamic setup of the data to be passed into the buffer one can declare the UBO as std140, and in this case the alignment of each array element is vec4 (according to the same section in the spec).

With that one could, for instance, use a shader like

static const char *fs_hack_gamma_frag_shader_src =
"#version 330\n"
"\n"
"uniform sampler2D tex;\n"
"in vec2 texCoord;\n"
"layout (std140) uniform ramp {\n"
"    vec3 values[256];\n"
"};\n"
"\n"
"layout(location = 0) out vec4 outColor;\n"
"\n"
"void main(void)\n"
"{\n"
"    vec4 lookup = texture(tex, texCoord) * 255.0;\n"
"    outColor.r = values[int(lookup.r)].x;\n"
"    outColor.g = values[int(lookup.g)].y;\n"
"    outColor.b = values[int(lookup.b)].z;\n"
"    outColor.a = 1.0;\n"
"}\n"
;

And pack the data passed into an array of 4 * 256 * sizeof(float) ordered [r,g,b,0].

gerddie avatar Jan 21 '22 09:01 gerddie

I was able to reproduce the issue directly on the hardware, i.e. a r600 based graphics card that is a vec4 based architecture.

gerddie avatar Jan 21 '22 10:01 gerddie

@gerddie Thanks for the report! We'll take a look at this soon. We're currently in the middle of a major Wine upgrade, so may take some time.

aeikum avatar Jan 21 '22 13:01 aeikum

Thanks, the changes based on this report are in Experimental ([bleeding edge] branch).

gofman avatar Jan 21 '22 18:01 gofman