wined3d: Fix regression from dynamic constants patch

Jason Green jave27 at gmail.com
Fri Jul 21 18:01:02 CDT 2006


I didn't notice before, but parts of the stateblock code use
memset/memcpy to set values on the StateBlock and the SAVEDSTATES.
When it is expecting a fixed array size, my constants patch introduced
some pointers instead.  So, the code was overwrite the pointers with
0x010101010, etc.  This situation never came up in my testing, but I
have found that Civ4 (and I'm sure others) use this codepath and would
no longer launch.

There were plenty of ways to fix this.  One would have been to unroll
all of the memset/memcpy calls into functions that explicitly set
every value on the stateblock.  While this might have been more
"correct", it would be harder to maintain.  Every time that someone
added something to the stateblock, those routines would have to be
updated.  Plus, we might lose some performance from numerous sets vs.
a single memset routine.

So, I chose to make the number of constants more fixed again (less
dynamic).  The number of available constants is now capped at 256,
which is the minimum required number for shader model 3.0.  The app
will use the min() of 256 and the number reported by the card (minus a
few implicit ones depending on the chosen shader model).

This patch also splits out the choosing of the max constants from
select_shader_mode() into its own function for organization purposes.
-------------- next part --------------
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 2f6c46a..6b3cc8d 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -781,7 +781,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl
     IWineD3DDeviceImpl     *This = (IWineD3DDeviceImpl *)iface;
     IWineD3DStateBlockImpl *object;
     int i, j;
-    HRESULT temp_result;
 
     D3DCREATEOBJECTINSTANCE(object, StateBlock)
     object->blockType     = Type;
@@ -794,10 +793,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl
         return WINED3D_OK;
     }
     
-    temp_result = allocate_shader_constants(object);
-    if (WINED3D_OK != temp_result)
-        return temp_result;
-
     /* Otherwise, might as well set the whole state block to the appropriate values  */
     if ( This->stateBlock != NULL) {
        memcpy(object, This->stateBlock, sizeof(IWineD3DStateBlockImpl));
@@ -5761,7 +5756,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl
 static HRESULT WINAPI IWineD3DDeviceImpl_BeginStateBlock(IWineD3DDevice *iface) {
     IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
     IWineD3DStateBlockImpl *object;
-    HRESULT temp_result;
     
     TRACE("(%p)", This);
     
@@ -5782,10 +5776,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl
     object->ref          = 1;
     object->lpVtbl       = &IWineD3DStateBlock_Vtbl;
     
-    temp_result = allocate_shader_constants(object);
-    if (WINED3D_OK != temp_result)
-        return temp_result;
-
     IWineD3DStateBlock_Release((IWineD3DStateBlock*)This->updateStateBlock);
     This->updateStateBlock = object;
     This->isRecordingState = TRUE;
diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c
index 35542af..5f7ac47 100644
--- a/dlls/wined3d/directx.c
+++ b/dlls/wined3d/directx.c
@@ -208,10 +208,6 @@ static void select_shader_mode(
     int* ps_selected,
     int* vs_selected) {
 
-    /* Default # of constants to 0 */
-    gl_info->max_vshader_constantsF = 0;
-    gl_info->max_pshader_constantsF = 0;
-
     /* Give priority to user disable/emulation request.
      * Then respect REF device for software.
      * Then check capabilities for hardware, and fallback to software */
@@ -222,12 +218,8 @@ static void select_shader_mode(
         *vs_selected = SHADER_SW;
     } else if (gl_info->supported[ARB_SHADING_LANGUAGE_100] && wined3d_settings.glslRequested) {
         *vs_selected = SHADER_GLSL;
-        /* Subtract the other potential uniforms from the max available (bools & ints) */
-        gl_info->max_vshader_constantsF = gl_info->vs_glsl_constantsF - MAX_CONST_B - MAX_CONST_I;
     } else if (gl_info->supported[ARB_VERTEX_PROGRAM]) {
         *vs_selected = SHADER_ARB;
-        /* ARB shaders seem to have an implicit PARAM when fog is used, so we need to subtract 1 from the total available */
-        gl_info->max_vshader_constantsF = gl_info->vs_arb_constantsF - 1;
     } else {
         *vs_selected = SHADER_SW;
     }
@@ -239,16 +231,53 @@ static void select_shader_mode(
         *ps_selected = SHADER_NONE;
     } else if (gl_info->supported[ARB_SHADING_LANGUAGE_100] && wined3d_settings.glslRequested) {
         *ps_selected = SHADER_GLSL;
-        /* Subtract the other potential uniforms from the max available (bools & ints) */
-        gl_info->max_pshader_constantsF = gl_info->ps_glsl_constantsF - MAX_CONST_B - MAX_CONST_I;
     } else if (gl_info->supported[ARB_FRAGMENT_PROGRAM]) {
         *ps_selected = SHADER_ARB;
-        gl_info->max_pshader_constantsF = gl_info->ps_arb_constantsF;
     } else {
         *ps_selected = SHADER_NONE;
     }
 }
 
+/** Select the number of report maximum shader constants based on the selected shader modes */
+void select_shader_max_constants(WineD3D_GL_Info *gl_info) {
+
+    switch (wined3d_settings.vs_selected_mode) {
+        case SHADER_GLSL:
+            /* Subtract the other potential uniforms from the max available (bools & ints) */
+            gl_info->max_vshader_constantsF = gl_info->vs_glsl_constantsF - MAX_CONST_B - MAX_CONST_I;
+            break;
+        case SHADER_ARB:
+            /* ARB shaders seem to have an implicit PARAM when fog is used, so we need to subtract 1 from the total available */
+            gl_info->max_vshader_constantsF = gl_info->vs_arb_constantsF - 1;
+            break;
+        case SHADER_SW:
+            gl_info->max_vshader_constantsF = 96;  /* TODO: Fixup software shaders */
+            break;
+        default:
+            gl_info->max_vshader_constantsF = 0;
+            break;
+    }
+
+    switch (wined3d_settings.ps_selected_mode) {
+        case SHADER_GLSL:
+            /* Subtract the other potential uniforms from the max available (bools & ints) */
+            gl_info->max_pshader_constantsF = gl_info->ps_glsl_constantsF - MAX_CONST_B - MAX_CONST_I;
+            break;
+        case SHADER_ARB:
+            gl_info->max_pshader_constantsF = gl_info->ps_arb_constantsF;
+            break;
+        case SHADER_SW:
+            gl_info->max_pshader_constantsF = 96;  /* TODO: Fixup software shaders */
+            break;
+        default:
+            gl_info->max_pshader_constantsF = 0;
+            break;
+    }
+
+    gl_info->max_vshader_constantsF = min(MAX_CONST_F, gl_info->max_vshader_constantsF);
+    gl_info->max_pshader_constantsF = min(MAX_CONST_F, gl_info->max_pshader_constantsF);
+}
+
 /**********************************************************
  * IWineD3D parts follows
  **********************************************************/
@@ -1528,6 +1557,7 @@ static HRESULT WINAPI IWineD3DImpl_GetDe
     }
     select_shader_mode(&This->gl_info, DeviceType,
         &wined3d_settings.ps_selected_mode, &wined3d_settings.vs_selected_mode);
+    select_shader_max_constants(&This->gl_info);
 
     /* ------------------------------------------------
        The following fields apply to both d3d8 and d3d9
@@ -1855,7 +1885,6 @@ static HRESULT  WINAPI IWineD3DImpl_Crea
     IWineD3DDeviceImpl *object  = NULL;
     IWineD3DImpl       *This    = (IWineD3DImpl *)iface;
     HDC hDC;
-    HRESULT temp_result;
 
     /* Validate the adapter number */
     if (Adapter >= IWineD3D_GetAdapterCount(iface)) {
@@ -1914,10 +1943,7 @@ static HRESULT  WINAPI IWineD3DImpl_Crea
     LEAVE_GL();
     select_shader_mode(&This->gl_info, DeviceType,
         &wined3d_settings.ps_selected_mode, &wined3d_settings.vs_selected_mode);
-
-    temp_result = allocate_shader_constants(object->updateStateBlock);
-    if (WINED3D_OK != temp_result)
-        return temp_result;
+    select_shader_max_constants(&This->gl_info);
 
     /* set the state of the device to valid */
     object->state = WINED3D_OK;
diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c
index e3d0e7c..ecdb89b 100644
--- a/dlls/wined3d/stateblock.c
+++ b/dlls/wined3d/stateblock.c
@@ -26,38 +26,6 @@ #include "wined3d_private.h"
 WINE_DEFAULT_DEBUG_CHANNEL(d3d);
 #define GLINFO_LOCATION ((IWineD3DImpl *)(((IWineD3DDeviceImpl *)This->wineD3DDevice)->wineD3D))->gl_info
 
-/***************************************
- * Stateblock helper functions follow
- **************************************/
-
-/** Allocates the correct amount of space for pixel and vertex shader constants, 
- * along with their set/changed flags on the given stateblock object
- */
-HRESULT allocate_shader_constants(IWineD3DStateBlockImpl* object) {
-    
-    IWineD3DStateBlockImpl *This = object;
-
-#define WINED3D_MEMCHECK(_object) if (NULL == _object) { FIXME("Out of memory!\n"); return E_OUTOFMEMORY; }
-
-    /* Allocate space for floating point constants */
-    object->pixelShaderConstantF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(float) * GL_LIMITS(pshader_constantsF) * 4);
-    WINED3D_MEMCHECK(object->pixelShaderConstantF);
-    object->set.pixelShaderConstantsF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(BOOL) * GL_LIMITS(pshader_constantsF) );
-    WINED3D_MEMCHECK(object->set.pixelShaderConstantsF);
-    object->changed.pixelShaderConstantsF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(BOOL) * GL_LIMITS(pshader_constantsF));
-    WINED3D_MEMCHECK(object->changed.pixelShaderConstantsF);
-    object->vertexShaderConstantF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(float) * GL_LIMITS(vshader_constantsF) * 4);
-    WINED3D_MEMCHECK(object->vertexShaderConstantF);
-    object->set.vertexShaderConstantsF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(BOOL) * GL_LIMITS(vshader_constantsF));
-    WINED3D_MEMCHECK(object->set.vertexShaderConstantsF);
-    object->changed.vertexShaderConstantsF = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(BOOL) * GL_LIMITS(vshader_constantsF));
-    WINED3D_MEMCHECK(object->changed.vertexShaderConstantsF);
-
-#undef WINED3D_MEMCHECK
-
-    return WINED3D_OK;
-}
-
 /**********************************************************
  * IWineD3DStateBlockImpl IUnknown parts follows
  **********************************************************/
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index efff579..7198237 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -53,8 +53,10 @@ #define MAX_ACTIVE_LIGHTS 8
 #define MAX_CLIPPLANES    D3DMAXUSERCLIPPLANES
 #define MAX_LEVELS        256
 
+/* Shader Constants - These are the minimum required for DX 9.0c cards */
 #define MAX_CONST_I 16
 #define MAX_CONST_B 16
+#define MAX_CONST_F 256
 
 /* Used for CreateStateBlock */
 #define NUM_SAVEDPIXELSTATES_R     35
@@ -1023,11 +1025,11 @@ typedef struct SAVEDSTATES {
         BOOL                      pixelShader;
         BOOL                      pixelShaderConstantsB[MAX_CONST_B];
         BOOL                      pixelShaderConstantsI[MAX_CONST_I];
-        BOOL                     *pixelShaderConstantsF;
+        BOOL                      pixelShaderConstantsF[MAX_CONST_F];
         BOOL                      vertexShader;
         BOOL                      vertexShaderConstantsB[MAX_CONST_B];
         BOOL                      vertexShaderConstantsI[MAX_CONST_I];
-        BOOL                     *vertexShaderConstantsF;
+        BOOL                      vertexShaderConstantsF[MAX_CONST_F];
 } SAVEDSTATES;
 
 struct IWineD3DStateBlockImpl
@@ -1055,7 +1057,7 @@ struct IWineD3DStateBlockImpl
     /* Vertex Shader Constants */
     BOOL                       vertexShaderConstantB[MAX_CONST_B];
     INT                        vertexShaderConstantI[MAX_CONST_I * 4];
-    float                     *vertexShaderConstantF;
+    float                      vertexShaderConstantF[MAX_CONST_F * 4];
 
     /* Stream Source */
     BOOL                      streamIsUP;
@@ -1091,7 +1093,7 @@ struct IWineD3DStateBlockImpl
     /* Pixel Shader Constants */
     BOOL                       pixelShaderConstantB[MAX_CONST_B];
     INT                        pixelShaderConstantI[MAX_CONST_I * 4];
-    float                     *pixelShaderConstantF;
+    float                      pixelShaderConstantF[MAX_CONST_F * 4];
 
     /* Indexed Vertex Blending */
     D3DVERTEXBLENDFLAGS       vertex_blend;
@@ -1417,8 +1419,6 @@ extern BOOL vshader_input_is_color(
     IWineD3DVertexShader* iface,
     unsigned int regnum);
 
-extern HRESULT allocate_shader_constants(IWineD3DStateBlockImpl* object);
-
 /* ARB_[vertex/fragment]_program helper functions */
 extern void shader_arb_load_constants(
     IWineD3DStateBlock* iface,


More information about the wine-patches mailing list