fix(shadertools): Correct order of uniforms in picking module

[felixpalmer]

Background

The order of uniforms in the uniformTypes must match that used in the shader. This was not the case in the picking module, which led to the incorrect layout being generated

with the positions of useFloatColors and isHighlightActive being swapped

Change List

• Correct order of uniforms in picking module

[donmccurdy]

The order of uniforms in the uniformTypes must match that used in the shader.

That sounds painful to discover! Do you know whether this is an intended constraint of ShaderInputs, or an unexpected one?

I wonder if we should include a (temporary?) lint before shader compilation to detect and warn about mismatched order. If it's an unintended or new constraint, there may be more cases like this.

[felixpalmer]

Do you know whether this is an intended constraint of ShaderInputs, or an unexpected one?

Not sure, @ibgreen?

I wonder if we should include a (temporary?) lint before shader compilation to detect and warn about mismatched order. If it's an unintended or new constraint, there may be more cases like this.

That would be nice, but not really sure how to go about it, and my current feeling is that we have a lot on our plate to land the picking UBO plate as is. If you want to take a stab though, that would be helpful :)

[donmccurdy]

Ah yeah! Just floating the idea, no expectation or ask on that. 😁 I may take a shot at it if I run into similar issues.

[ibgreen]

Do you know whether this is an intended constraint of ShaderInputs, or an unexpected one?

It is fundamental, unrelated to ShaderInputs. The memory layout is defined by the order of the declarations in the uniform block in the shader code, and that binary layout must be precisely replicated on the CPU in the block that is passed in.

luma.gl does extract some (currently unused) information about uniform blocks using the WebGL introspection API so it should be fairly easy to write some code that checks that the layouts are identical.

Another option is to autogenerate the uniform block declarations in the shaders from the shader module uniform declarations which would guaranteed that types are always matching.

But that would be a pretty big "philosophy" change: luma.gl has been focused on enabling users to write their own shaders (as opposed to shaders being something that is magically generated by the engine).

I'd prefer to have multiple reasons for making such a big change, rather than just trying to prevent one type of error.