3

I am working on a C++ program which displays a terrain mesh using GLSL shaders. I want it to be able to use different materials based on the elevation.

I am trying to accomplish this by having an uniform array of materials in the fragment shader and then using the y coordinate of the world-space position of the current fragment to determine which material from the array to use.

Here are the relevant parts of my fragment shader:

#version 430

struct Material
{
    vec3 ambient;
    vec3 diffuse;
    vec3 specular;
    int shininess;
    sampler2D diffuseTex;
    bool hasDiffuseTex;
    float maxY; //the upper bound of this material's layer in relation to the height of the mesh (in the range 0-1)
};

in vec2 TexCoords;
in vec3 WorldPos;

const int MAX_MATERIALS = 14;

uniform Material materials[MAX_MATERIALS];
uniform int materialCount; //the actual number of materials in the array

uniform float minY; //the minimum world-space y-coordinate in the mesh
uniform float maxY; //the maximum world-space y-coordinate in the mesh

out vec4 fragColor;

void main()
{

    //calculate the y-position of this fragment in relation to the height of the mesh (in the range 0-1)
    float y = (WorldPos.y - minY) / (maxY - minY);

    //calculate the index into the materials array 
    int index = 0;
    for (int i = 0; i < materialCount; ++i)
    {
        index += int(y > materials[i].maxY);
    }

    //calculate the ambient color
    vec3 ambient = ...

    //calculate the diffuse color
    vec3 diffuse = ...
    //sample from the texture
    vec3 texColor = vec3(texture(materials[index].diffuseTex, TexCoords.xy));
    //only multiply diffuse color with texture color if the material has a texture
    diffuse += int(materials[index].hasDiffuseTex) * ((texColor * diffuse) - diffuse);

    //calculate the specular color
    vec3 specular = ...

    fragColor = vec4(ambient + diffuse + specular, 1.0f);
}

It works fine if textures are not used:

Mesh without texture

But if one of the materials has a texture associated with it, it shows some black artifacts near the borders of the material layer which has the texture:

Mesh with texture

When I add this line after the diffuse calculation part:

if (index == 0 && int(materials[index].hasDiffuseTex) == 1 && texColor == vec3(0, 0, 0)) diffuse = vec3(1, 0, 0);

it draws the artifacts in red:

Mesh with texture 2

which tells me that the index is correct (0) but nothing is sampled from the texture.

Furthermore if I hardcode the index into the shader like this:

vec3 texColor = vec3(texture(materials[0].diffuseTex, TexCoords.xy));

it renders correctly. So I am guessing it has something to do with the indexing but the index appears to be correct and the texture is there so why doesn't it sample color?

I have also found out that if I switch the order of the materials and move their borders around in the GUI of my program in a certain fashion it starts to render correctly from that point on which I don't understand at all. I first suspected that this might be due to me sending wrong values of uniforms to the shaders initially and then somehow it gets the correct ones after I make the changes in the GUI but then I have tested all the uniform values I am sending to the shader from the C++ side and they all appear to be correct from the start and I don't see any other possible problem which might cause this from the C++ side. So I am now thinking the problem is probably in the shader.

Scheff's Cat
  • 19,528
  • 6
  • 28
  • 56
Stibius
  • 93
  • 6
  • My guess is that the OpenGL implementation you're using does not like when you sample textures dynamically, as there are probably several optimizations based on the assumption that the first argument to `texture` is always [dynamically uniform](https://www.khronos.org/opengl/wiki/Core_Language_(GLSL)#Dynamically_uniform_expression). Even if your implementation did support that process correctly, choosing samplers based on fragment data is really bad for shader performance. – Magma Jul 21 '19 at 23:41
  • I suggest that you replace the first argument to `texture` with a dynamically uniform expression, most preferably a constant. In particular, I recommend that you use a single texture to store all materials at once, either side-by-side as a stitched atlas, in which case you can look up subtexture sampling offsets from a uniform offset array, or as a 3D texture with one layer per material, if you need the materials to wrap. – Magma Jul 21 '19 at 23:41
  • I'm not sure but I don't think you should be multiplying an integer by a vec3 `int(materials[index].hasDiffuseTex) * ((texColor * diffuse) - diffuse);` Did you want a step function? – Wyck Jul 21 '19 at 23:42
  • 1
    @Wyck Since `int(boolvar)` evaluates to 1 if `boolvar` is true and 0 otherwise, the idiom `int(boolvar) * value` is a non-branching alternative to `boolvar ? value : 0` which improves performance in not-so-smart GLSL compilers. Casting the boolean to a different primitive type is necessary since you can't multiply by booleans directly. The integer will be immediately converted to float though, so it is probably more appropriate to cast it to float directly. – Magma Jul 21 '19 at 23:48
  • FYI, when it comes to multitexturing, the hardware will sample all textures anyway. Just make sure that the texture coordinates you use to sample the textures you are not using do not vary so that it uses the cached texture value (e.g. sample textures you are not using with coords (0,0)). This will not actually result in a subsequent memory read believe it or not (after the first one), so sample all 14 textures at once and then just "select" (by linear combination) which values to mix in with non-zero multipliers. Will likely be just as performant as what you are attempting; plus no stalls. – Wyck Jul 21 '19 at 23:59
  • @Wyck I tried the "sample all textures" approach and it solved the problem. So the problem indeed was that I was sampling textures dynamically. – Stibius Jul 22 '19 at 00:21
  • Possible duplicate of [Odd behavior of GLSL switch statement](https://stackoverflow.com/questions/41694335/odd-behavior-of-glsl-switch-statement) – BDL Jul 22 '19 at 08:42
  • 1
    @Wyck: "*the hardware will sample all textures anyway*" No, it won't. – Nicol Bolas Jul 22 '19 at 13:28
  • @NicolBolas, 600 chars isn't enough for me to explain, but I was referring to using multitexturing instead of dynamic texturing as a potential workaround heeding Magma's advice that choosing samplers based on fragment data is a bad idea. One alternative is to use all the samplers, rather than choose one from them -- i.e. multitexturing. My advice was how to avoid reading from 14 textures when you only want to sample 1: the trick to gain performance is to access the same texture coordinates on every unselected texture. Leveraging the cache like this turns 14 reads/pixel into 1 read/pixel. – Wyck Jul 22 '19 at 14:17

0 Answers0