Regression in generation of symbols for ShaderGen Type Globals

[kwokcb]

Issue

The logic to generate globals appears to have "broken" sometime between Nov 15 and 29,2203 -- at least for Python libraries but perhaps for shared libraries in general.

This shows up for example in this code for GLSL uniforms.
The code perform pointers comparisons which can now fail when running outside the test suite.

int GlslProgram::mapTypeToOpenGLType(const TypeDesc* type)
{
    if (type == Type::INTEGER)
        return GL_INT;
    else if (type == Type::BOOLEAN) <-- can fail
        return GL_BOOL;
    else if (type == Type::FLOAT) <-- can fail
        return GL_FLOAT;
    else if (type->isFloat2())
        return GL_FLOAT_VEC2;
    else if (type->isFloat3())
        return GL_FLOAT_VEC3;
    else if (type->isFloat4())
        return GL_FLOAT_VEC4;
    else if (type == Type::MATRIX33) <-- can fail
        return GL_FLOAT_MAT3;
    else if (type == Type::MATRIX44) <-- can fail
        return GL_FLOAT_MAT4;
    else if (type == Type::FILENAME)
    {
        // A "filename" is not indicative of type, so just return a 2d sampler.
        return GL_SAMPLER_2D;
    }

    return GlslProgram::Input::INVALID_OPENGL_TYPE;
}

You get these errors when trying to render from Python:

Errors: "GLSL uniform parsing error
Vertex shader uniform block type mismatch [PrivateUniforms]. Name: "u_worldMatrix". Type: "matrix44". Semantic: "". Value: "<none>". Unit: "<none>". Colorspace: "<none>". GLType: -1
Vertex shader uniform block type mismatch [PrivateUniforms]. Name: "u_viewProjectionMatrix". Type: "matrix44". Semantic: "". Value: "<none>". Unit: "<none>". Colorspace: "<none>". GLType: -1"

The root issue appears that globals defined for Type's get duplicated per Python module. This can be checked by adding debugging statements to Type constructor here: https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/MaterialXGenShader/TypeDesc.cpp#L42

This does not appear to be cause by this module dependency change: #1595
so it is still unknown why this regression has occurred.

The simplest test is to add debugging prints to the global declaration initializations and having a Python file which does the following:

import MaterialX.PyMaterialXGenShader as mx_gen_shader
import MaterialX.PyMaterialXGenGlsl as mx_gen_glsl

Globals appear to be initialized twice -- once for each module.

[kwokcb]

This is a workaround for the compare, but there could be issue elsewhere for global pointer compares:

int GlslProgram::mapTypeToOpenGLType(const TypeDesc* type)
{
    if (type == Type::INTEGER)
        return GL_INT;
    else if (type->getName() == Type::BOOLEAN->getName())
        return GL_BOOL;
    else if (type->getName() == Type::FLOAT->getName())
        return GL_FLOAT;
    else if (type->isFloat2())
        return GL_FLOAT_VEC2;
    else if (type->isFloat3())
        return GL_FLOAT_VEC3;
    else if (type->isFloat4())
        return GL_FLOAT_VEC4;
    else if (type->getName() == Type::MATRIX33->getName())
        return GL_FLOAT_MAT3;
    else if (type->getName() == Type::MATRIX44->getName())
    {
        return GL_FLOAT_MAT4;
    } 
    else if (type == Type::FILENAME)
    {
        // A "filename" is not indicative of type, so just return a 2d sampler.
        return GL_SAMPLER_2D;
    }

    return GlslProgram::Input::INVALID_OPENGL_TYPE;
}

[jstone-lucasfilm]

Thanks for writing this up, @kwokcb, and it sounds like a key issue to investigate and/or fix before we wrap up MaterialX 1.38.9.

[niklasharrysson]

As we discussed offline the source of this issue seems to be the use of global pointers to identify the registered TypeDesc instances. It's not safe to use the pointers as unique identifiers when used over module boundaries. At least not in a Python context.

In hindsight this is not a good design in general, and we should rewamp the TypeDesc class in a future release when API breakage is ok. The short term fix will be to change the TypeDesc compare operations to use the type name strings instead.

[jstone-lucasfilm]

I'll mark this as resolved, based on your fix in #1665, with the note that @niklasharrysson is planning a more thorough refactoring of the TypeDesc system in MaterialX 1.39.