Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-77665

Phong fragment shader does not account correctly for ambient color

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P3: Somewhat important
    • 6.7.0 Beta2, 6.8.0 FF
    • 5.13.0
    • Qt3D
    • None
    • 491e25e10 (dev), 9cc525731 (6.7)

    Description

      The phong fragment shader `phong.inc.frag` in `extras/shaders/gl3`, does not correctly handle the ambient color portion according to the phong model.

      A good link explaining the formula for phong shading is for example:

      http://what-when-how.com/opengl-programming-guide/the-mathematics-of-lighting-opengl-programming/

      Especially, the following line in `phong.inc.frag` seems to be very unlikely to be correct:

          // Combine spec with ambient+diffuse for final fragment color
          vec3 color = (ambient.rgb + diffuseColor) * diffuse.rgb
                     + specularColor * specular.rgb;
      
      

      If one factor out the above line, especially the prodcut

      ambient.rgb*diffuse.rgb 
      

      does not look very convincing. There is no light factor after all.

      A more sound solution correctly handling ambient color portion might be given by the following file:

       #pragma include light.inc.frag
      
          void adsModel(const in vec3 worldPos,
                        const in vec3 worldNormal,
                        const in vec3 worldView,
                        const in float shininess,
                        out vec3 ambientColor, // ADDED
                        out vec3 diffuseColor,
                        out vec3 specularColor)
          {
              diffuseColor = vec3(0.0);
              specularColor = vec3(0.0);
              ambientColor = vec3(0.0);
      
              // We perform all work in world space
              vec3 n = normalize(worldNormal);
              vec3 s = vec3(0.0);
      
              for (int i = 0; i < lightCount; ++i) {
                  float att = 1.0;
                  float sDotN = 0.0;
      
                  if (lights[i].type != TYPE_DIRECTIONAL) {
                      // Point and Spot lights
      
                      // Light position is already in world space
                      vec3 sUnnormalized = lights[i].position - worldPos;
                      s = normalize(sUnnormalized); // Light direction
      
                      // Calculate the attenuation factor
                      sDotN = dot(s, n);
                      if (sDotN > 0.0) {
                          if (lights[i].constantAttenuation != 0.0
                           || lights[i].linearAttenuation != 0.0
                           || lights[i].quadraticAttenuation != 0.0) {
                              float dist = length(sUnnormalized);
                              att = 1.0 / (lights[i].constantAttenuation +
                                           lights[i].linearAttenuation * dist +
                                           lights[i].quadraticAttenuation * dist * dist);
                          }
      
                          // The light direction is in world space already
                          if (lights[i].type == TYPE_SPOT) {
                              // Check if fragment is inside or outside of the spot light cone
                              if (degrees(acos(dot(-s, lights[i].direction))) > lights[i].cutOffAngle)
                                  sDotN = 0.0;
                          }
                      }
                  } else {
                      // Directional lights
                      // The light direction is in world space already
                      s = normalize(-lights[i].direction);
                      sDotN = dot(s, n);
                  }
      
                  // Calculate the diffuse factor
                  float diffuse = max(sDotN, 0.0);
      
                  // Calculate the specular factor
                  float specular = 0.0;
                  if (diffuse > 0.0 && shininess > 0.0) {
                      float normFactor = (shininess + 2.0) / 2.0;
                      vec3 r = reflect(-s, n);   // Reflection direction in world space
                      specular = normFactor * pow(max(dot(r, worldView), 0.0), shininess);
                  }
      
                  // Accumulate the diffuse and specular contributions
                  ambientColor + =att * lights[i].intensity * 1.0 * lights[i].color; // ADDED
                  diffuseColor += att * lights[i].intensity * diffuse * lights[i].color;
                  specularColor += att * lights[i].intensity * specular * lights[i].color;
              }
          }
      
          vec4 phongFunction(const in vec4 ambient,
                             const in vec4 diffuse,
                             const in vec4 specular,
                             const in float shininess,
                             const in vec3 worldPosition,
                             const in vec3 worldView,
                             const in vec3 worldNormal)
          {
              // Calculate the lighting model, keeping the specular component separate
              vec3 diffuseColor, specularColor;
              adsModel(worldPosition, worldNormal, worldView, shininess, ambientColor, diffuseColor, specularColor); // CHANGED
      
              vec3 color = ambientColor * ambient.rgb + diffuseColor * diffuse.rgb
                         + specularColor * specular.rgb;  /// CHANGED
      
              return vec4(color, diffuse.a);
          }
      
      

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              seanharmer Sean Harmer
              aleph0 Frank Simon
              Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes