Fix GLES3 multimesh rendering when using colors or custom data

[bitsawer]

• Fixes OpenGL: The last instance of MultiMesh incorrectly sets the color #71897

OpenGL compatibility renderer compresses the 4-float instance color and instance custom data into 2 floats each, which means that if you only use color OR custom data the component count given to glVertexAttribIPointer() will be wrong. This causes the last rendered element to access data outside the instance buffer and the value it will try to read will be invalid (or it might crash your driver if you're unlucky).

This PR adjusts the component count to use 2 if only color or custom data are used. If both are used, the count will be 4.

[clayjohn]

I think this won't work in the case where only a custom value is sent. The color is always in .xy and the custom is always in .zw. If only a custom value is used, the .zw values will be ignored and the user will get garbage

[bitsawer]

Good catch, the demo project didn't test that scenario so I missed that. I think the additional fix should be pretty simple, but I'll test this some more.

[bitsawer]

I dived into a rabbit hole chasing that "easy" fix, turns out the perfect fix for this issue is pretty complex and probably not worth it in code maintainability terms. I also noticed that the scenario clayjohn mentioned (GLES3 multimesh with disabled instance colors but enabled custom data) is also broken in master, but I guess it's a rare usecase so nobody has reported it. I implemented a simpler fix, which adds a small amount of memory padding in some cases. I'll explain my reasoning here.

Current MultiMesh 3D mode instance data configurations and layout in the master. Data is grouped as vec4/uvec4 input attributes:

1. (MMMM)(MMMM)(MMMM)        (only matrix, 12 floats as three vec4's, works)
2. (MMMM)(MMMM)(MMMM)(CC  )  (matrix + color, last item's color broken in current master)
3. (MMMM)(MMMM)(MMMM)(AA  )  (matrix + custom attribute, fully broken in current master)
4. (MMMM)(MMMM)(MMMM)(CCAA)  (matrix + color + custom, works)

Note that the memory layout doesn't always match what we tell to OpenGL and what we use in our GLSL code, which breaks rendering in two configurations (2 and 3). After this PR, the memory layout looks like this and always matches the GLSL input attributes and access:

1. (MMMM)(MMMM)(MMMM)        (only matrix, 12 floats, no change)
2. (MMMM)(MMMM)(MMMM)(CC..)  (matrix + color, added 8 bytes of padding)
3. (MMMM)(MMMM)(MMMM)(..AA)  (matrix + custom attribute, added 8 bytes of padding)
4. (MMMM)(MMMM)(MMMM)(CCAA)  (matrix + color + custom, no change)

Aside from mismatched layout, another problem with the current behavior is that in some configurations it tries to force what is basically uvec2 data into uvec4 input attribute and gives wrong component count to OpenGL when calling glVertexAttribIPointer(), which also causes all kinds of issues.

To fix this without padding, we would have to add a large amount of conditional macros to scene.glsl, canvas.glsl and particles.glsl. Each place would have a lot of conditional compilation branches to make sure that input attribute data type (uvec2 or uvec4) and access patterns (using .xy or .zw) change depending on how the multimesh is configured. It would make those parts of the shader code very unreadable and harder to maintain (and it might also create more shader compilation variants).

I feel that always forcing color and custom instance data into uvec4 layout is a reasonably good way to fix this issue. If we want to optimize this, we can always do it later.

[clayjohn]

I agree with your reasoning. Its not ideal to always include padding, but realistically, the driver is likely going to add padding when we give it an attribute with only 2 components anyway.

[YuriSizov]

Thanks!

[YuriSizov]

Cherry-picked for 4.1.2.