fix [1610840] Positional light doesn't work correctly on emulator

This bug was introduced when lighting computations was changed from eye-space to object-space.
The light position need to be transformed back to object-space each time the modelview matrix changes which requires us to compute the inverse of the modelview matrix. This computation was done with the assumption that normals where transformed (which was the case when the computation was made in eye-space), however, normals only require the inverse of the upper 3x3 matrix while transforming positions requires the inverse of the whole matrix.
This caused the interesting behavior that lights were more-or-less transformed properly, but not translated at all, which caused improper lighting with directional lights in particular.

There was also another smaller bug affecting directional lights: when vertices are read, only the active component are read, the other ones are ignored, later, the transformation operations are set up to ignore the unset values, howver, in the case of lighting, we use the vertex in object space (that is, before it is transformed), and therefore were using uninitalized values; in particular w.

3 files changed, 39 insertions, 78 deletions

diff --git a/opengl/libagl/array.cpp b/opengl/libagl/array.cpp
index 8fa7566..3e9c6a5 100644
--- a/opengl/libagl/array.cpp
+++ b/opengl/libagl/array.cpp
@@ -951,6 +951,8 @@ void compileElement__generic(ogles_context_t* c,
     v->index = first;
     first &= vertex_cache_t::INDEX_MASK;
     const GLubyte* vp = c->arrays.vertex.element(first);
+    v->obj.z = 0;
+    v->obj.w = 0x10000;
     c->arrays.vertex.fetch(c, v->obj.v, vp);
     c->arrays.mvp_transform(&c->transforms.mvp, &v->clip, &v->obj);
     c->arrays.perspective(c, v);
@@ -966,6 +968,8 @@ void compileElements__generic(ogles_context_t* c,
     do {
         v->flags = 0;
         v->index = first++;
+        v->obj.z = 0;
+        v->obj.w = 0x10000;
         c->arrays.vertex.fetch(c, v->obj.v, vp);
         c->arrays.mvp_transform(mvp, &v->clip, &v->obj);
         c->arrays.perspective(c, v);
diff --git a/opengl/libagl/light.cpp b/opengl/libagl/light.cpp
index bc9449c..8ae32cc0f 100644
--- a/opengl/libagl/light.cpp
+++ b/opengl/libagl/light.cpp
@@ -38,13 +38,14 @@ static void lightVertex(ogles_context_t* c, vertex_t* v);
 static void lightVertexMaterial(ogles_context_t* c, vertex_t* v);
 
 static inline void vscale3(GLfixed* d, const GLfixed* m, GLfixed s);
-static inline void vsub3w(GLfixed* d, const GLfixed* a, const GLfixed* b);
 
 static __attribute__((noinline))
 void vnorm3(GLfixed* d, const GLfixed* a);
 
 static inline void vsa3(GLfixed* d,
     const GLfixed* m, GLfixed s, const GLfixed* a);
+static inline void vss3(GLfixed* d,
+    const GLfixed* m, GLfixed s, const GLfixed* a);
 static inline void vmla3(GLfixed* d,
     const GLfixed* m0, const GLfixed* m1, const GLfixed* a);
 static inline void vmul3(GLfixed* d,
@@ -151,18 +152,10 @@ void vsa3(GLfixed* d, const GLfixed* m, GLfixed s, const GLfixed* a) {
 }
 
 static inline
-void vsub3w(GLfixed* d, const GLfixed* a, const GLfixed* b) {
-    const GLfixed wa = a[3];
-    const GLfixed wb = b[3];
-    if (ggl_likely(wa == wb)) {
-        d[0] = a[0] - b[0];
-        d[1] = a[1] - b[1];
-        d[2] = a[2] - b[2];
-    } else {
-        d[0] = gglMulSubx(a[0], wb, gglMulx(b[0], wa));
-        d[1] = gglMulSubx(a[1], wb, gglMulx(b[1], wa));
-        d[2] = gglMulSubx(a[2], wb, gglMulx(b[2], wa));
-    }
+void vss3(GLfixed* d, const GLfixed* m, GLfixed s, const GLfixed* a) {
+    d[0] = gglMulSubx(m[0], s, a[0]);
+    d[1] = gglMulSubx(m[1], s, a[1]);
+    d[2] = gglMulSubx(m[2], s, a[2]);
 }
 
 static inline
@@ -227,7 +220,7 @@ static inline void validate_light_mvi(ogles_context_t* c)
         const int i = 31 - gglClz(en);
         en &= ~(1<<i);
         light_t& l = c->lighting.lights[i];
-        c->transforms.mvui.point3(&c->transforms.mvui,
+        c->transforms.mvui.point4(&c->transforms.mvui,
                 &l.objPosition, &l.position);
         vnorm3(l.normalizedObjPosition.v, l.objPosition.v);
     }
@@ -348,7 +341,11 @@ void lightVertex(ogles_context_t* c, vertex_t* v)
         vec4_t n;
         c->arrays.normal.fetch(c, n.v,
             c->arrays.normal.element(v->index & vertex_cache_t::INDEX_MASK));
-        if (c->transforms.rescaleNormals == GL_NORMALIZE)
+
+        // TODO: right now we handle GL_RESCALE_NORMALS as if ti were
+        // GL_NORMALIZE. We could optimize this by  scaling mvui 
+        // appropriately instead.
+        if (c->transforms.rescaleNormals)
             vnorm3(n.v, n.v);
 
         const material_t& material = c->lighting.front;
@@ -365,7 +362,8 @@ void lightVertex(ogles_context_t* c, vertex_t* v)
 
             // compute vertex-to-light vector
             if (ggl_unlikely(l.position.w)) {
-                vsub3w(d.v, l.objPosition.v, v->obj.v);
+                // lightPos/1.0 - vertex/vertex.w == lightPos*vertex.w - vertex
+                vss3(d.v, l.objPosition.v, v->obj.w, v->obj.v);
                 sqDist = dot3(d.v, d.v);
                 vscale3(d.v, d.v, gglSqrtRecipx(sqDist));
             } else {
diff --git a/opengl/libagl/matrix.cpp b/opengl/libagl/matrix.cpp
index f175cda..0b68dc0 100644
--- a/opengl/libagl/matrix.cpp
+++ b/opengl/libagl/matrix.cpp
@@ -55,7 +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 normal__generic(transform_t const*, vec4_t* c, vec4_t const* o);
+static void point4__mvui(transform_t const*, vec4_t* c, vec4_t const* o);
 
 // ----------------------------------------------------------------------------
 #if 0
@@ -209,7 +209,8 @@ void mvui_transform_t::picker()
 {
     flags = 0;
     ops = OP_ALL;
-    point3 = normal__generic;
+    point3 = point4__mvui;
+    point4 = point4__mvui;
 }
 
 void transform_t::dump(const char* what)
@@ -596,66 +597,19 @@ void transform_state_t::update_mvit()
 
 void transform_state_t::update_mvui()
 {
+    GLfloat r[16];
     const GLfloat* const mv = modelview.top().elements();
-
-    /*
-    When transforming normals, we can use the upper 3x3 matrix, see:
-    http://www.opengl.org/documentation/specs/version1.1/glspec1.1/node26.html
-    */
     
-    // Also note that:
-    //      l(obj) =  tr(M).l(eye) for infinite light
-    //      l(obj) = inv(M).l(eye) for local light
-
-    const uint32_t ops = modelview.top_ops() & ~OP_TRANSLATE;
-    if (ggl_likely((!(ops & ~OP_ROTATE)) ||
-        (rescaleNormals && modelview.isRigidBody()))) {
-        // if the modelview matrix is a rigid body transformation
-        // (translation, rotation, uniform scaling), then we can bypass
-        // the inverse by transposing the matrix.
-        GLfloat rescale = 1.0f;
-        if (rescaleNormals == GL_RESCALE_NORMAL) {
-            if (!(ops & ~OP_UNIFORM_SCALE)) {
-                rescale = reciprocalf(mv[I(0,0)]);
-            } else {
-                rescale = rsqrtf(
-                        sqrf(mv[I(2,0)]) + sqrf(mv[I(2,1)]) + sqrf(mv[I(2,2)]));
-            }
-        }
-        GLfixed* const x = mvui.matrix.m;
-        for (int i=0 ; i<3 ; i++) {
-            x[I(i,0)] = gglFloatToFixed(mv[I(0,i)] * rescale);
-            x[I(i,1)] = gglFloatToFixed(mv[I(1,i)] * rescale);
-            x[I(i,2)] = gglFloatToFixed(mv[I(2,i)] * rescale);
-        }
-        mvui.picker();
-        return;
-    }
-
-    GLfloat r[3][3];
-    r[0][0] = det22(mv[I(1,1)], mv[I(2,1)], mv[I(1,2)], mv[I(2,2)]);
-    r[0][1] =ndet22(mv[I(0,1)], mv[I(2,1)], mv[I(0,2)], mv[I(2,2)]);
-    r[0][2] = det22(mv[I(0,1)], mv[I(1,1)], mv[I(0,2)], mv[I(1,2)]);
-    r[1][0] =ndet22(mv[I(1,0)], mv[I(2,0)], mv[I(1,2)], mv[I(2,2)]);
-    r[1][1] = det22(mv[I(0,0)], mv[I(2,0)], mv[I(0,2)], mv[I(2,2)]);
-    r[1][2] =ndet22(mv[I(0,0)], mv[I(1,0)], mv[I(0,2)], mv[I(1,2)]);
-    r[2][0] = det22(mv[I(1,0)], mv[I(2,0)], mv[I(1,1)], mv[I(2,1)]);
-    r[2][1] =ndet22(mv[I(0,0)], mv[I(2,0)], mv[I(0,1)], mv[I(2,1)]);
-    r[2][2] = det22(mv[I(0,0)], mv[I(1,0)], mv[I(0,1)], mv[I(1,1)]);        
-
-    GLfloat rdet;
-    if (rescaleNormals == GL_RESCALE_NORMAL) {
-        rdet = rsqrtf(sqrf(r[0][2]) + sqrf(r[1][2]) + sqrf(r[2][2]));
-    } else {
-        rdet = reciprocalf( 
-            r[0][0]*mv[I(0,0)] + r[0][1]*mv[I(1,0)] + r[0][2]*mv[I(2,0)]);
-    }
+    // TODO: we need a faster invert, especially for when the modelview
+    // is a rigid-body matrix
+    invert(r, mv);
 
     GLfixed* const x = mvui.matrix.m;
-    for (int i=0 ; i<3 ; i++) {
-        x[I(i,0)] = gglFloatToFixed(r[i][0] * rdet);
-        x[I(i,1)] = gglFloatToFixed(r[i][1] * rdet);
-        x[I(i,2)] = gglFloatToFixed(r[i][2] * rdet);
+    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)]);
     }
     mvui.picker();
 }
@@ -783,14 +737,19 @@ 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 normal__generic(transform_t const* mx, vec4_t* lhs, vec4_t const* rhs) {
+void point4__mvui(transform_t const* mx, vec4_t* lhs, vec4_t const* rhs) {
+    // this used for transforming light positions back to object space.
+    // Lights have 3 components positions, so w is always 1.
+    // however, it 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->x = mla3a(rx, m[ 0], ry, m[ 4], rz, m[ 8], m[12]); 
+    lhs->y = mla3a(rx, m[ 1], ry, m[ 5], rz, m[ 9], m[13]);
+    lhs->z = mla3a(rx, m[ 2], ry, m[ 6], rz, m[10], m[14]);
+    lhs->w = rhs->w;
 }