fix [2133133] Software OpenGL ES Lighting is buggy (GL Gears washed out bug)

A typo caused GL_AMBIENT_AND_DIFFUSE to only set the the ambient color.

Fix another typo which caused the viewer position to be wrong for
specular highlights.

Switch back to eye-space lighting, since there are still some issues
with some demos (San Angeles in particular).

3 files changed, 78 insertions, 26 deletions

diff --git a/opengl/libagl/light.cpp b/opengl/libagl/light.cpp
index f211bca..ca715db 100644
--- a/opengl/libagl/light.cpp
+++ b/opengl/libagl/light.cpp
@@ -217,18 +217,26 @@ static inline void validate_light_mvi(ogles_context_t* c)
 {
     uint32_t en = c->lighting.enabledLights;
     // Vector from object to viewer, in eye coordinates
-    const vec4_t eyeViewer = { 0, 0, 0x1000, 0 };
     while (en) {
         const int i = 31 - gglClz(en);
         en &= ~(1<<i);
         light_t& l = c->lighting.lights[i];
+#if OBJECT_SPACE_LIGHTING
         c->transforms.mvui.point4(&c->transforms.mvui,
                 &l.objPosition, &l.position);
+#else
+        l.objPosition = l.position;
+#endif
         vnorm3(l.normalizedObjPosition.v, l.objPosition.v);
-        c->transforms.mvui.point4(&c->transforms.mvui,
-                &l.objViewer, &eyeViewer);
-        vnorm3(l.objViewer.v, l.objViewer.v);
     }
+    const vec4_t eyeViewer = { 0, 0, 0x10000, 0 };
+#if OBJECT_SPACE_LIGHTING
+    c->transforms.mvui.point3(&c->transforms.mvui,
+            &c->lighting.objViewer, &eyeViewer);
+    vnorm3(c->lighting.objViewer.v, c->lighting.objViewer.v);
+#else
+    c->lighting.objViewer = eyeViewer;
+#endif
 }
 
 static inline void validate_light(ogles_context_t* c)
@@ -337,6 +345,7 @@ void lightVertex(ogles_context_t* c, vertex_t* v)
 {
     // emission and ambient for the whole scene
     vec4_t r = c->lighting.implicitSceneEmissionAndAmbient;
+    const vec4_t objViewer = c->lighting.objViewer;
 
     uint32_t en = c->lighting.enabledLights;
     if (ggl_likely(en)) {
@@ -347,7 +356,11 @@ void lightVertex(ogles_context_t* c, vertex_t* v)
         c->arrays.normal.fetch(c, n.v,
             c->arrays.normal.element(v->index & vertex_cache_t::INDEX_MASK));
 
-        // TODO: right now we handle GL_RESCALE_NORMALS as if ti were
+#if !OBJECT_SPACE_LIGHTING
+        c->transforms.mvui.point3(&c->transforms.mvui, &n, &n);
+#endif
+
+        // TODO: right now we handle GL_RESCALE_NORMALS as if it were
         // GL_NORMALIZE. We could optimize this by  scaling mvui 
         // appropriately instead.
         if (c->transforms.rescaleNormals)
@@ -380,13 +393,13 @@ void lightVertex(ogles_context_t* c, vertex_t* v)
             s = dot3(n.v, d.v);
             s = (s<0) ? (twoSide?(-s):0) : s;
             vsa3(t.v, l.implicitDiffuse.v, s, l.implicitAmbient.v);
-            
+
             // specular
             if (ggl_unlikely(s && l.implicitSpecular.v[3])) {
                 vec4_t h;
-                h.x = d.x + l.objViewer.x;
-                h.y = d.y + l.objViewer.y;
-                h.z = d.z + l.objViewer.z;
+                h.x = d.x + objViewer.x;
+                h.y = d.y + objViewer.y;
+                h.z = d.z + objViewer.z;
                 vnorm3(h.v, h.v);
                 s = dot3(n.v, h.v);
                 s = (s<0) ? (twoSide?(-s):0) : s;
@@ -515,15 +528,18 @@ static void lightxv(GLenum i, GLenum pname, const GLfixed *params, ogles_context
     case GL_POSITION: {
         ogles_validate_transform(c, transform_state_t::MODELVIEW);
         transform_t& mv = c->transforms.modelview.transform;
-        memcpy(light.position.v, params, sizeof(light.position.v));
-        mv.point4(&mv, &light.position, &light.position);
+        mv.point4(&mv, &light.position, reinterpret_cast<vec4_t const*>(params));
         invalidate_lighting(c);
         return;
     }
     case GL_SPOT_DIRECTION: {
+#if OBJECT_SPACE_LIGHTING
         ogles_validate_transform(c, transform_state_t::MVUI);
         transform_t& mvui = c->transforms.mvui;
-        mvui.point3(&mvui, &light.spotDir, (vec4_t*)params);
+        mvui.point3(&mvui, &light.spotDir, reinterpret_cast<vec4_t const*>(params));
+#else
+        light.spotDir = *reinterpret_cast<vec4_t const*>(params);
+#endif
         vnorm3(light.normalizedSpotDir.v, light.spotDir.v);
         invalidate_lighting(c);
         return;
@@ -748,8 +764,8 @@ void glMaterialfv(
     case GL_SPECULAR:   what = c->lighting.front.specular.v; break;
     case GL_EMISSION:   what = c->lighting.front.emission.v; break;
     case GL_AMBIENT_AND_DIFFUSE:
-        what  = c->lighting.front.ambient.v; break;
-        other = c->lighting.front.diffuse.v; break;
+        what  = c->lighting.front.ambient.v;
+        other = c->lighting.front.diffuse.v;
         break;
     case GL_SHININESS:
         c->lighting.front.shininess = gglFloatToFixed(params[0]);
@@ -788,8 +804,8 @@ void glMaterialxv(
     case GL_SPECULAR:   what = c->lighting.front.specular.v; break;
     case GL_EMISSION:   what = c->lighting.front.emission.v; break;
     case GL_AMBIENT_AND_DIFFUSE:
-        what = c->lighting.front.ambient.v; break;
-        other= c->lighting.front.diffuse.v; break;
+        what  = c->lighting.front.ambient.v;
+        other = c->lighting.front.diffuse.v;
         break;
     case GL_SHININESS:
         c->lighting.front.shininess = gglFloatToFixed(params[0]);
diff --git a/opengl/libagl/light.h b/opengl/libagl/light.h
index 6dae25f..39e3309 100644
--- a/opengl/libagl/light.h
+++ b/opengl/libagl/light.h
@@ -22,6 +22,13 @@
 #include <stddef.h>
 #include <sys/types.h>
 
+
+// Set to 1 for object-space lighting evaluation.
+// There are still some bugs with object-space lighting,
+// especially visible in the San Angeles demo.
+#define OBJECT_SPACE_LIGHTING   0
+
+
 namespace android {
 
 namespace gl {
diff --git a/opengl/libagl/matrix.cpp b/opengl/libagl/matrix.cpp
index 3c50977..9520f04 100644
--- a/opengl/libagl/matrix.cpp
+++ b/opengl/libagl/matrix.cpp
@@ -55,6 +55,7 @@ static void normal__nop(transform_t const*, vec4_t* c, vec4_t const* o);
 static void point2__generic(transform_t const*, vec4_t* c, vec4_t const* o);
 static void point3__generic(transform_t const*, vec4_t* c, vec4_t const* o);
 static void point4__generic(transform_t const*, vec4_t* c, vec4_t const* o);
+static void point3__mvui(transform_t const*, vec4_t* c, vec4_t const* o);
 static void point4__mvui(transform_t const*, vec4_t* c, vec4_t const* o);
 
 // ----------------------------------------------------------------------------
@@ -209,7 +210,7 @@ void mvui_transform_t::picker()
 {
     flags = 0;
     ops = OP_ALL;
-    point3 = point4__mvui;
+    point3 = point3__mvui;
     point4 = point4__mvui;
 }
 
@@ -600,17 +601,31 @@ void transform_state_t::update_mvui()
     GLfloat r[16];
     const GLfloat* const mv = modelview.top().elements();
     
-    // TODO: we need a faster invert, especially for when the modelview
-    // is a rigid-body matrix
+    /*
+    When evaluating the lighting equation in eye-space, normals
+    are transformed by the upper 3x3 modelview inverse-transpose.
+    http://www.opengl.org/documentation/specs/version1.1/glspec1.1/node26.html
+
+    (note that inverse-transpose is distributive).
+    Also note that:
+        l(obj) = inv(modelview).l(eye) for local light
+        l(obj) =  tr(modelview).l(eye) for infinite light
+    */
+
     invert(r, mv);
 
     GLfixed* const x = mvui.matrix.m;
-    for (int i=0 ; i<4 ; i++) {
-        x[I(i,0)] = gglFloatToFixed(r[I(i,0)]);
-        x[I(i,1)] = gglFloatToFixed(r[I(i,1)]);
-        x[I(i,2)] = gglFloatToFixed(r[I(i,2)]);
-        x[I(i,4)] = gglFloatToFixed(r[I(i,3)]);
-    }
+
+#if OBJECT_SPACE_LIGHTING
+    for (int i=0 ; i<4 ; i++)
+        for (int j=0 ; j<4 ; j++)
+            x[I(i,j)] = gglFloatToFixed(r[I(i,j)]);
+#else
+    for (int i=0 ; i<4 ; i++)
+        for (int j=0 ; j<4 ; j++)
+            x[I(i,j)] = gglFloatToFixed(r[I(j,i)]);
+#endif
+
     mvui.picker();
 }
 
@@ -739,8 +754,22 @@ void point4__generic(transform_t const* mx, vec4_t* lhs, vec4_t const* rhs) {
     lhs->w = mla4(rx, m[ 3], ry, m[ 7], rz, m[11], rw, m[15]);
 }
 
+void point3__mvui(transform_t const* mx, vec4_t* lhs, vec4_t const* rhs) {
+    // this is used for transforming light positions back to object space.
+    // w is used as a switch for directional lights, so we need
+    // to preserve it.
+    const GLfixed* const m = mx->matrix.m;
+    const GLfixed rx = rhs->x;
+    const GLfixed ry = rhs->y;
+    const GLfixed rz = rhs->z;
+    lhs->x = mla3(rx, m[ 0], ry, m[ 4], rz, m[ 8]);
+    lhs->y = mla3(rx, m[ 1], ry, m[ 5], rz, m[ 9]);
+    lhs->z = mla3(rx, m[ 2], ry, m[ 6], rz, m[10]);
+    lhs->w = 0;
+}
+
 void point4__mvui(transform_t const* mx, vec4_t* lhs, vec4_t const* rhs) {
-    // this used for transforming light positions back to object space.
+    // this is used for transforming light positions back to object space.
     // w is used as a switch for directional lights, so we need
     // to preserve it.
     const GLfixed* const m = mx->matrix.m;