in Direct3D, Report vertex shader compile errors, make RSQ work with either version of swizzle code

* * richardvoigt at gmail.com
Tue Jan 2 12:26:53 CST 2007


This patch repairs the broken translation of reciprocal square root
(RSQ) in shader programs which was creating lines like:

RSQ R1.x, R0.xxxx

which are rejected by the compiler (the input to RSQ is a scalar,
swizzle notation isn't allowed) at least with my ndivia driver.  Every
example of RSQ I could find anywhere uses just a single component, and
it fixes Galactic Civilizations 2 "Dark Avatar" which I'm trying to
make run.

This along with implementation of ScriptStringOut makes GC2 quite
playable (though the save menu still crashes).

Hey, just noticed that the swizzle code was reverted in the tree,
breaking my patch.... so now I have two, to make it work either way.
Some of my changes aren't needed with the old version of the swizzle
code, I guess.  I still think there's some value to the other changes
which report an error when the shader compilation fails.
-------------- next part --------------
From a26509e096617cbbb52eb4dba36ab6666cf21a8b Mon Sep 17 00:00:00 2001
From: Richard Voigt richardvoigt at gmail.com <ben at gondor.lan>
Date: Tue, 2 Jan 2007 12:04:52 -0600
Subject: Propagate vertex shader compile errors, don't invoke shader when compile failed.
Also, fix generate of RSQ calls (RSQ takes a scalar input and the compiler rejects swizzle syntax)
---
 dlls/wined3d/arb_program_shader.c |   16 +++++++++--
 dlls/wined3d/drawprim.c           |   52 ++++++++++++++++++++-----------------
 dlls/wined3d/pixelshader.c        |    7 +++--
 dlls/wined3d/vertexshader.c       |   22 ++++++++++-----
 dlls/wined3d/wined3d_private.h    |    8 +++++-
 5 files changed, 66 insertions(+), 39 deletions(-)

diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c
index a8e4c9a..a44a123 100644
--- a/dlls/wined3d/arb_program_shader.c
+++ b/dlls/wined3d/arb_program_shader.c
@@ -300,7 +300,12 @@ static void pshader_get_register_name(
 }
 
 /* TODO: merge with pixel shader */
-static void vshader_program_add_param(SHADER_OPCODE_ARG *arg, const DWORD param, BOOL is_input, char *hwLine) {
+static void vshader_program_add_param(SHADER_OPCODE_ARG *arg, const DWORD param, BOOL is_input, UINT scalar_input, char *hwLine) {
+/* scalar_input: 0 -> vector */
+/*               1 -> opcode uses .x only */
+/*               2 -> opcode uses .y only */
+/*               3 -> opcode uses .z only */
+/*               4 -> opcode uses .w only (for instance RSQ) */
 
   IWineD3DVertexShaderImpl* This = (IWineD3DVertexShaderImpl*) arg->shader;
 
@@ -367,6 +372,11 @@ static void vshader_program_add_param(SH
   } else {
     char swizzle[6];
     shader_arb_get_swizzle(param, is_color, swizzle);
+    if (scalar_input > 0) {
+        /* swizzle[0] == '.' */
+        swizzle[1] = swizzle[scalar_input];
+        swizzle[2] = 0;
+    }
     strcat(hwLine, swizzle);
   }
 }
@@ -872,10 +882,10 @@ void vshader_hw_map2gl(SHADER_OPCODE_ARG
         strcpy(tmpLine, curOpcode->glname);
 
     if (curOpcode->num_params > 0) {
-        vshader_program_add_param(arg, dst, FALSE, tmpLine);
+        vshader_program_add_param(arg, dst, FALSE, 0, tmpLine);
         for (i = 1; i < curOpcode->num_params; ++i) {
            strcat(tmpLine, ",");
-           vshader_program_add_param(arg, src[i-1], TRUE, tmpLine);
+           vshader_program_add_param(arg, src[i-1], TRUE, (curOpcode->opcode == WINED3DSIO_RSQ? 4: 0), tmpLine);
         }
     }
    shader_addline(buffer, "%s;\n", tmpLine);
diff --git a/dlls/wined3d/drawprim.c b/dlls/wined3d/drawprim.c
index 63e1cae..08fe1dd 100644
--- a/dlls/wined3d/drawprim.c
+++ b/dlls/wined3d/drawprim.c
@@ -1009,11 +1009,9 @@ static void loadVertexData(IWineD3DDevic
 
     /* Point Size ----------------------------------------------*/
     if (sd->u.s.pSize.lpData || sd->u.s.pSize.VBO) {
-
         /* no such functionality in the fixed function GL pipeline */
         TRACE("Cannot change ptSize here in openGl\n");
         /* TODO: Implement this function in using shaders if they are available */
-
     }
 
     /* Vertex Pointers -----------------------------------------*/
@@ -1229,13 +1227,14 @@ static void drawStridedFast(IWineD3DDevi
 #endif
         glDrawElements(glPrimitiveType, numberOfVertices, idxSize == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT,
                      (const char *)idxData+(idxSize * startIdx));
+        checkGLcall("glDrawElements");
 #else /* using drawRangeElements may be faster */
 
         glDrawRangeElements(glPrimitiveType, minIndex, minIndex + numberOfVertices - 1, numberOfVertices,
                       idxSize == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT,
                       (const char *)idxData+(idxSize * startIdx));
-#endif
         checkGLcall("glDrawRangeElements");
+#endif
 
     } else {
 
@@ -1253,7 +1252,6 @@ static void drawStridedFast(IWineD3DDevi
  * Actually draw using the supplied information.
  * Slower GL version which extracts info about each vertex in turn
  */
-	
 static void drawStridedSlow(IWineD3DDevice *iface, WineDirect3DVertexStridedData *sd,
                      UINT NumVertexes, GLenum glPrimType,
                      const void *idxData, short idxSize, ULONG minIndex, ULONG startIdx) {
@@ -1346,7 +1344,6 @@ static void drawStridedSlow(IWineD3DDevi
 
         /* Point Size ----------------------------- */
         if (sd->u.s.pSize.lpData != NULL) {
-
             float *ptrToCoords = (float *)(sd->u.s.pSize.lpData + (SkipnStrides * sd->u.s.pSize.dwStride));
             ptSize = ptrToCoords[0];
             VTRACE(("ptSize=%f\n", ptSize));
@@ -1740,43 +1737,50 @@ inline static void drawPrimitiveDrawStri
 #define BUFFER_OR_DATA(_attribute) dataLocations->u.s._attribute.lpData
     /* TODO: Either support missing functionality in fixupVertices or by creating a shader to replace the pipeline. */
     if (!useVertexShaderFunction && (BUFFER_OR_DATA(blendMatrixIndices) || BUFFER_OR_DATA(blendWeights))) {
-        FIXME("Blending data is only valid with vertex shaders %p %p\n",dataLocations->u.s.blendWeights.lpData,dataLocations->u.s.blendWeights.lpData);
+        FIXME("Blending data is only valid with vertex shaders %p %p\n",BUFFER_OR_DATA(blendMatrixIndices),BUFFER_OR_DATA(blendWeights));
     }
     if (!useVertexShaderFunction && (BUFFER_OR_DATA(position2) || BUFFER_OR_DATA(normal2))) {
-        FIXME("Tweening is only valid with vertex shaders\n");
+        FIXME("Tweening is only valid with vertex shaders %p %p\n", BUFFER_OR_DATA(position2), BUFFER_OR_DATA(normal2));
     }
     if (!useVertexShaderFunction && (BUFFER_OR_DATA(tangent) || BUFFER_OR_DATA(binormal))) {
-        FIXME("Tangent and binormal bump mapping is only valid with vertex shaders\n");
+        FIXME("Tangent and binormal bump mapping is only valid with vertex shaders %p %p\n", BUFFER_OR_DATA(tangent), BUFFER_OR_DATA(binormal));
     }
     if (!useVertexShaderFunction && (BUFFER_OR_DATA(tessFactor) || BUFFER_OR_DATA(fog) || BUFFER_OR_DATA(depth) || BUFFER_OR_DATA(sample))) {
-        FIXME("Extended attributes are only valid with vertex shaders\n");
+        FIXME("Extended attributes are only valid with vertex shaders %p %p %p %p\n", BUFFER_OR_DATA(tessFactor), BUFFER_OR_DATA(fog), BUFFER_OR_DATA(depth), BUFFER_OR_DATA(sample));
     }
 #undef BUFFER_OR_DATA
 
+    /* Draw vertex by vertex */
+    if(useVertexShaderFunction) {
+    	HRESULT hrCompile;
+        /* We compile the shader here because we need the vertex declaration
+         * in order to determine if we need to do any swizzling for D3DCOLOR
+         * registers. If the shader is already compiled this call will do nothing. */
+        hrCompile = IWineD3DVertexShader_CompileShader(This->stateBlock->vertexShader);
+        TRACE("(%p) Shader compile complete -- %s\n", This, FAILED(hrCompile)? "w/errors": "success");
+        if (FAILED(hrCompile)) {
+            return;
+        }
+        else {
+            TRACE("(%p) Loading numbered arrays, using compiled shader\n", This);
+            loadNumberedArrays(iface, This->stateBlock->vertexShader, dataLocations);
+            useDrawStridedSlow = FALSE;
+        }
+    }
+    else
     /* Fixed pipeline, no fixups required - load arrays */
-    if (!useVertexShaderFunction &&
-       ((dataLocations->u.s.pSize.lpData == NULL &&
+    if ((dataLocations->u.s.pSize.lpData == NULL &&
          dataLocations->u.s.diffuse.lpData == NULL &&
          dataLocations->u.s.specular.lpData == NULL) ||
-         fixup) ) {
+            fixup) {
 
         /* Load the vertex data using named arrays */
         TRACE("(%p) Loading vertex data\n", This);
         loadVertexData(iface, dataLocations);
         useDrawStridedSlow = FALSE;
-
+    }
     /* Shader pipeline - load attribute arrays */
-    } else if(useVertexShaderFunction) {
-
-        loadNumberedArrays(iface, This->stateBlock->vertexShader, dataLocations);
-        useDrawStridedSlow = FALSE;
-
-        /* We compile the shader here because we need the vertex declaration
-         * in order to determine if we need to do any swizzling for D3DCOLOR
-         * registers. If the shader is already compiled this call will do nothing. */
-        IWineD3DVertexShader_CompileShader(This->stateBlock->vertexShader);
-    /* Draw vertex by vertex */
-    } else { 
+    else { 
         TRACE("Not loading vertex data\n");
         useDrawStridedSlow = TRUE;
     }
diff --git a/dlls/wined3d/pixelshader.c b/dlls/wined3d/pixelshader.c
index 862d56a..870d6df 100644
--- a/dlls/wined3d/pixelshader.c
+++ b/dlls/wined3d/pixelshader.c
@@ -962,11 +962,12 @@ static HRESULT WINAPI IWineD3DPixelShade
     TRACE("(%p) : function %p\n", iface, function);
 
     /* We're already compiled. */
-    if (This->baseShader.is_compiled) return WINED3D_OK;
+    if (This->baseShader.is_compiled == SHADER_COMPILED_OK) return WINED3D_OK;
+    if (This->baseShader.is_compiled == SHADER_COMPILE_ERRORS) return WINED3DERR_INVALIDCALL;
 
     /* We don't need to compile */
     if (!function || This->baseShader.shader_mode == SHADER_SW) {
-        This->baseShader.is_compiled = TRUE;
+        This->baseShader.is_compiled = SHADER_COMPILED_OK;
         return WINED3D_OK;
     }
 
@@ -981,7 +982,7 @@ static HRESULT WINAPI IWineD3DPixelShade
     TRACE("(%p) : Generating hardware program\n", This);
     IWineD3DPixelShaderImpl_GenerateShader(iface, &This->baseShader.reg_maps, function);
 
-    This->baseShader.is_compiled = TRUE;
+    This->baseShader.is_compiled = SHADER_COMPILED_OK;
 
     return WINED3D_OK;
 }
diff --git a/dlls/wined3d/vertexshader.c b/dlls/wined3d/vertexshader.c
index f68a2fb..46b0262 100644
--- a/dlls/wined3d/vertexshader.c
+++ b/dlls/wined3d/vertexshader.c
@@ -693,7 +693,7 @@ BOOL vshader_input_is_color(
 
 /** Generate a vertex shader string using either GL_VERTEX_PROGRAM_ARB
     or GLSL and send it to the card */
-static VOID IWineD3DVertexShaderImpl_GenerateShader(
+static BOOL IWineD3DVertexShaderImpl_GenerateShader(
     IWineD3DVertexShader *iface,
     shader_reg_maps* reg_maps,
     CONST DWORD *pFunction) {
@@ -820,15 +820,16 @@ static VOID IWineD3DVertexShaderImpl_Gen
         if (glGetError() == GL_INVALID_OPERATION) {
             GLint errPos;
             glGetIntegerv(GL_PROGRAM_ERROR_POSITION_ARB, &errPos);
-            FIXME("HW VertexShader Error at position %d: %s\n",
-                  errPos, debugstr_a((const char *)glGetString(GL_PROGRAM_ERROR_STRING_ARB)));
+            FIXME("HW VertexShader Error at position: %d\n%s\n", errPos, debugstr_a((const char*)glGetString(GL_PROGRAM_ERROR_STRING_ARB)));
             This->baseShader.prgId = -1;
+            return FALSE;
         }
     }
 
 #if 1 /* if were using the data buffer of device then we don't need to free it */
   HeapFree(GetProcessHeap(), 0, buffer.buffer);
 #endif
+    return TRUE;
 }
 
 BOOL IWineD3DVertexShaderImpl_ExecuteHAL(IWineD3DVertexShader* iface, WINEVSHADERINPUTDATA* input, WINEVSHADEROUTPUTDATA* output) {
@@ -885,7 +886,7 @@ HRESULT WINAPI IWineD3DVertexShaderImpl_
     TRACE_VSVECTOR(input->V[D3DVSDE_TEXCOORD1]);
 #endif
 
-    TRACE_VSVECTOR(vshader->data->C[64]);
+    TRACE_VSVECTOR(This->data->C[64]);
     /* TODO: Run through all the tokens and find and labels, if, endifs, loops etc...., and make a labels list */
 
     /* the first dword is the version tag */
@@ -1234,19 +1235,24 @@ static HRESULT WINAPI IWineD3DVertexShad
     TRACE("(%p) : function %p\n", iface, function);
 
     /* We're already compiled. */
-    if (This->baseShader.is_compiled) return WINED3D_OK;
+    if (This->baseShader.is_compiled == SHADER_COMPILED_OK) return WINED3D_OK;
+    if (This->baseShader.is_compiled == SHADER_COMPILE_ERRORS) return WINED3DERR_INVALIDCALL;
 
     /* We don't need to compile */
     if (!function || This->baseShader.shader_mode == SHADER_SW) {
-        This->baseShader.is_compiled = TRUE;
+        This->baseShader.is_compiled = SHADER_COMPILED_OK;
         return WINED3D_OK;
     }
 
     /* Generate the HW shader */
     TRACE("(%p) : Generating hardware program\n", This);
-    IWineD3DVertexShaderImpl_GenerateShader(iface, &This->baseShader.reg_maps, function);
+    if (!IWineD3DVertexShaderImpl_GenerateShader(iface, &This->baseShader.reg_maps, function)) {
+        This->baseShader.is_compiled = SHADER_COMPILE_ERRORS;
+
+        return WINED3DERR_INVALIDCALL;
+    }
 
-    This->baseShader.is_compiled = TRUE;
+    This->baseShader.is_compiled = SHADER_COMPILED_OK;
 
     return WINED3D_OK;
 }
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index a6fe6c5..6f52926 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -1662,6 +1662,12 @@ extern void vshader_glsl_output_unpack(
    SHADER_BUFFER* buffer,
    semantic* semantics_out);
 
+enum shader_is_compiled_t {
+	SHADER_NOT_COMPILED = 0,
+	SHADER_COMPILED_OK,
+	SHADER_COMPILE_ERRORS
+};
+
 /*****************************************************************************
  * IDirect3DBaseShader implementation structure
  */
@@ -1674,7 +1680,7 @@ typedef struct IWineD3DBaseShaderClass
     CONST DWORD                     *function;
     UINT                            functionLength;
     GLuint                          prgId;
-    BOOL                            is_compiled;
+    enum shader_is_compiled_t       is_compiled;
 
     /* Type of shader backend */
     int shader_mode;
-- 
1.4.4.1
-------------- next part --------------
From c4bc8e896b2dda5f4641e60484ef0cff40cf3ebb Mon Sep 17 00:00:00 2001
From: Richard Voigt richardvoigt at gmail.com <ben at gondor.lan>
Date: Tue, 2 Jan 2007 12:26:44 -0600
Subject: Make RSQ fix work with either version of swizzle code

---
 dlls/wined3d/arb_program_shader.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c
index a44a123..6a9fc7b 100644
--- a/dlls/wined3d/arb_program_shader.c
+++ b/dlls/wined3d/arb_program_shader.c
@@ -372,7 +372,7 @@ static void vshader_program_add_param(SH
   } else {
     char swizzle[6];
     shader_arb_get_swizzle(param, is_color, swizzle);
-    if (scalar_input > 0) {
+    if (scalar_input > 0 && strlen(swizzle) > 2) {
         /* swizzle[0] == '.' */
         swizzle[1] = swizzle[scalar_input];
         swizzle[2] = 0;
-- 
1.4.4.1


More information about the wine-patches mailing list