[WINED3D] Clean up some shader issues

Ivan Gyurdiev ivg231 at gmail.com
Sun Sep 17 17:53:29 CDT 2006


Issue #1) I am seeing a lot of cases where the shader opcodes walk up 
the tree to the shader device pointer, and then back down to the 
stateblock, to look at necessary states. This is a bad design pattern, 
and leads to hidden dependencies on the stateblock. We want to be 
passing resources we use from the caller down, not querying them from 
the callee by upwards pointers.

Issue #2) I found a place where the base shader pointer was being casted 
to pixel to look at the wineD3DDevice field. This only works by chance - 
the field should be stored and read from the parent class 
(IWineD3DBaseShaderClass).

Changes:
- move device pointer from pixel/vertex objects into parent baseshader class
- store stateblock and device directly into SHADER_OPCODE_ARG structure, 
and read them from there, instead of following upwards references
- fix some wrong comments in the tex instruction

-------------- next part --------------
---
 dlls/wined3d/arb_program_shader.c |    3 ++-
 dlls/wined3d/baseshader.c         |    4 ++++
 dlls/wined3d/device.c             |   14 ++++++++++++--
 dlls/wined3d/glsl_shader.c        |   12 ++++++------
 dlls/wined3d/pixelshader.c        |   10 ++++++----
 dlls/wined3d/vertexshader.c       |   13 ++++++++-----
 dlls/wined3d/wined3d_private.h    |    8 ++++++--
 7 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c
index e984b9d..f5af897 100644
--- a/dlls/wined3d/arb_program_shader.c
+++ b/dlls/wined3d/arb_program_shader.c
@@ -614,6 +614,7 @@ void pshader_hw_map2gl(SHADER_OPCODE_ARG
 void pshader_hw_tex(SHADER_OPCODE_ARG* arg) {
 
     IWineD3DPixelShaderImpl* This = (IWineD3DPixelShaderImpl*) arg->shader;
+    IWineD3DStateBlockImpl* stateBlock = (IWineD3DStateBlockImpl*) arg->stateBlock;
     DWORD dst = arg->dst;
     DWORD* src = arg->src;
     SHADER_BUFFER* buffer = arg->buffer;
@@ -667,7 +668,7 @@ void pshader_hw_tex(SHADER_OPCODE_ARG* a
          tex_type = "2D";
   }
 
-  if(This->wineD3DDevice->stateBlock->textureState[reg_sampler_code][D3DTSS_TEXTURETRANSFORMFLAGS] & D3DTTFF_PROJECTED) {
+  if(stateBlock->textureState[reg_sampler_code][D3DTSS_TEXTURETRANSFORMFLAGS] & D3DTTFF_PROJECTED) {
       shader_addline(buffer, "TXP %s, %s, texture[%lu], %s;\n",
           reg_dest, reg_coord, reg_sampler_code, tex_type);
   } else {
diff --git a/dlls/wined3d/baseshader.c b/dlls/wined3d/baseshader.c
index ebcc160..a608051 100644
--- a/dlls/wined3d/baseshader.c
+++ b/dlls/wined3d/baseshader.c
@@ -675,6 +675,8 @@ void shader_generate_main(
     CONST DWORD* pFunction) {
 
     IWineD3DBaseShaderImpl* This = (IWineD3DBaseShaderImpl*) iface;
+    IWineD3DDevice* device = This->baseShader.device;
+    IWineD3DDeviceImpl* deviceImpl = (IWineD3DDeviceImpl*) device;
     const DWORD *pToken = pFunction;
     const SHADER_OPCODE *curOpcode = NULL;
     SHADER_HANDLER hw_fct = NULL;
@@ -683,6 +685,8 @@ void shader_generate_main(
 
     /* Initialize current parsing state */
     hw_arg.shader = iface;
+    hw_arg.device = device;
+    hw_arg.stateBlock = (IWineD3DStateBlock*) deviceImpl->stateBlock;
     hw_arg.buffer = buffer;
     hw_arg.reg_maps = reg_maps;
     This->baseShader.parse_state.current_row = 0;
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 34026db..45e22de 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -95,6 +95,16 @@ #define D3DCREATEOBJECTINSTANCE(object, 
     *pp##type = (IWineD3D##type *) object; \
 }
 
+#define D3DCREATESHADEROBJECTINSTANCE(object, type) { \
+    object=HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(IWineD3D##type##Impl)); \
+    D3DMEMCHECK(object, pp##type); \
+    object->lpVtbl = &IWineD3D##type##_Vtbl;  \
+    object->parent       = parent; \
+    object->ref          = 1; \
+    object->baseShader.device = (IWineD3DDevice*) This; \
+    *pp##type = (IWineD3D##type *) object; \
+}
+
 #define  D3DCREATERESOURCEOBJECTINSTANCE(object, type, d3dtype, _size){ \
     object=HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(IWineD3D##type##Impl)); \
     D3DMEMCHECK(object, pp##type); \
@@ -1932,7 +1942,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl
     IWineD3DDeviceImpl       *This = (IWineD3DDeviceImpl *)iface;
     IWineD3DVertexShaderImpl *object;  /* NOTE: impl usage is ok, this is a create */
     HRESULT hr = WINED3D_OK;
-    D3DCREATEOBJECTINSTANCE(object, VertexShader)
+    D3DCREATESHADEROBJECTINSTANCE(object, VertexShader)
     object->baseShader.shader_ins = IWineD3DVertexShaderImpl_shader_ins;
 
     TRACE("(%p) : Created Vertex shader %p\n", This, *ppVertexShader);
@@ -1977,7 +1987,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl
     IWineD3DPixelShaderImpl *object; /* NOTE: impl allowed, this is a create */
     HRESULT hr = WINED3D_OK;
 
-    D3DCREATEOBJECTINSTANCE(object, PixelShader)
+    D3DCREATESHADEROBJECTINSTANCE(object, PixelShader)
     object->baseShader.shader_ins = IWineD3DPixelShaderImpl_shader_ins;
     hr = IWineD3DPixelShader_SetFunction(*ppPixelShader, pFunction);
     if (WINED3D_OK == hr) {
diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c
index 257b9e5..902faf7 100644
--- a/dlls/wined3d/glsl_shader.c
+++ b/dlls/wined3d/glsl_shader.c
@@ -586,7 +586,8 @@ static void shader_glsl_get_register_nam
     /* oPos, oFog and oPts in D3D */
     const char* hwrastout_reg_names[] = { "gl_Position", "gl_FogFragCoord", "gl_PointSize" };
 
-    WineD3D_GL_Info *gl_info = &((IWineD3DImpl*)((IWineD3DPixelShaderImpl*)arg->shader)->wineD3DDevice->wineD3D)->gl_info;
+    IWineD3DDeviceImpl* deviceImpl = (IWineD3DDeviceImpl*) arg->device;
+    WineD3D_GL_Info *gl_info = &((IWineD3DImpl*)deviceImpl->wineD3D)->gl_info;
     DWORD reg = param & D3DSP_REGNUM_MASK;
     DWORD regtype = shader_get_regtype(param);
     IWineD3DBaseShaderImpl* This = (IWineD3DBaseShaderImpl*) arg->shader;
@@ -1343,9 +1344,8 @@ void shader_glsl_callnz(SHADER_OPCODE_AR
  ********************************************/
 void pshader_glsl_tex(SHADER_OPCODE_ARG* arg) {
 
-    /* FIXME: Make this work for more than just 2D textures */
-    
     IWineD3DPixelShaderImpl* This = (IWineD3DPixelShaderImpl*) arg->shader;
+    IWineD3DStateBlockImpl* stateBlockImpl = (IWineD3DStateBlockImpl*) arg->stateBlock;
     SHADER_BUFFER* buffer = arg->buffer;
     DWORD hex_version = This->baseShader.hex_version;
 
@@ -1365,8 +1365,8 @@ void pshader_glsl_tex(SHADER_OPCODE_ARG*
     else
        shader_glsl_add_param(arg, arg->src[0], arg->src_addr[0], TRUE, coord_reg, coord_mask, coord_str);
 
-    /* 1.0-1.4: Use destination register as coordinate source.
-     * 2.0+: Use provided coordinate source register. */
+    /* 1.0-1.4: Use destination register for sampler index
+     * 2.0+: Use provided sampler. */
     if (hex_version < D3DPS_VERSION(2,0)) {
         sprintf(sampler_str, "Psampler%lu", reg_dest_code); 
         sampler_code = reg_dest_code;
@@ -1377,7 +1377,7 @@ void pshader_glsl_tex(SHADER_OPCODE_ARG*
     }         
 
     sampler_type = arg->reg_maps->samplers[sampler_code] & WINED3DSP_TEXTURETYPE_MASK;
-    if(This->wineD3DDevice->stateBlock->textureState[sampler_code][D3DTSS_TEXTURETRANSFORMFLAGS] & D3DTTFF_PROJECTED) {
+    if(stateBlockImpl->textureState[sampler_code][D3DTSS_TEXTURETRANSFORMFLAGS] & D3DTTFF_PROJECTED) {
         switch(sampler_type) {
 
             case WINED3DSTT_2D:
diff --git a/dlls/wined3d/pixelshader.c b/dlls/wined3d/pixelshader.c
index 814c40e..98ec7d7 100644
--- a/dlls/wined3d/pixelshader.c
+++ b/dlls/wined3d/pixelshader.c
@@ -31,7 +31,7 @@ #include "wined3d_private.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(d3d_shader);
 
-#define GLINFO_LOCATION ((IWineD3DImpl *)(((IWineD3DDeviceImpl *)This->wineD3DDevice)->wineD3D))->gl_info
+#define GLINFO_LOCATION ((IWineD3DImpl *)(((IWineD3DDeviceImpl *)This->baseShader.device)->wineD3D))->gl_info
 
 #if 0 /* Must not be 1 in cvs version */
 # define PSTRACE(A) TRACE A
@@ -102,8 +102,8 @@ static HRESULT  WINAPI IWineD3DPixelShad
 
 static HRESULT  WINAPI IWineD3DPixelShaderImpl_GetDevice(IWineD3DPixelShader* iface, IWineD3DDevice **pDevice){
     IWineD3DPixelShaderImpl *This = (IWineD3DPixelShaderImpl *)iface;
-    IWineD3DDevice_AddRef((IWineD3DDevice *)This->wineD3DDevice);
-    *pDevice = (IWineD3DDevice *)This->wineD3DDevice;
+    IWineD3DDevice_AddRef(This->baseShader.device);
+    *pDevice = This->baseShader.device;
     TRACE("(%p) returning %p\n", This, *pDevice);
     return WINED3D_OK;
 }
@@ -944,7 +944,9 @@ static HRESULT WINAPI IWineD3DPixelShade
 }
 
 static HRESULT WINAPI IWineD3DPixelShaderImpl_CompileShader(IWineD3DPixelShader *iface) {
+
     IWineD3DPixelShaderImpl *This =(IWineD3DPixelShaderImpl *)iface;
+    IWineD3DDeviceImpl *deviceImpl = (IWineD3DDeviceImpl*) This->baseShader.device;
     CONST DWORD *function = This->baseShader.function;
     shader_reg_maps *reg_maps = &This->baseShader.reg_maps;
     HRESULT hr;
@@ -963,7 +965,7 @@ static HRESULT WINAPI IWineD3DPixelShade
     /* Second pass: figure out which registers are used, what the semantics are, etc.. */
     memset(reg_maps, 0, sizeof(shader_reg_maps));
     hr = shader_get_registers_used((IWineD3DBaseShader*) This, reg_maps,
-                                    This->semantics_in, NULL, This->baseShader.function, This->wineD3DDevice->stateBlock);
+        This->semantics_in, NULL, This->baseShader.function, deviceImpl->stateBlock);
     if (hr != WINED3D_OK) return hr;
     /* FIXME: validate reg_maps against OpenGL */
 
diff --git a/dlls/wined3d/vertexshader.c b/dlls/wined3d/vertexshader.c
index 9719264..35637fd 100644
--- a/dlls/wined3d/vertexshader.c
+++ b/dlls/wined3d/vertexshader.c
@@ -31,7 +31,7 @@ #include "wined3d_private.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(d3d_shader);
 
-#define GLINFO_LOCATION ((IWineD3DImpl *)(((IWineD3DDeviceImpl *)This->wineD3DDevice)->wineD3D))->gl_info
+#define GLINFO_LOCATION ((IWineD3DImpl *)(((IWineD3DDeviceImpl *)This->baseShader.device)->wineD3D))->gl_info
 
 /* Shader debugging - Change the following line to enable debugging of software
       vertex shaders                                                             */
@@ -659,6 +659,8 @@ BOOL vshader_input_is_color(
     unsigned int regnum) {
 
     IWineD3DVertexShaderImpl* This = (IWineD3DVertexShaderImpl*) iface;
+    IWineD3DDeviceImpl* deviceImpl = (IWineD3DDeviceImpl*) This->baseShader.device;
+
     DWORD usage_token = This->semantics_in[regnum].usage;
     DWORD usage = (usage_token & D3DSP_DCL_USAGE_MASK) >> D3DSP_DCL_USAGE_SHIFT;
     DWORD usage_idx = (usage_token & D3DSP_DCL_USAGEINDEX_MASK) >> D3DSP_DCL_USAGEINDEX_SHIFT;
@@ -669,7 +671,7 @@ BOOL vshader_input_is_color(
         vertexDeclaration = (IWineD3DVertexDeclarationImpl *)This->vertexDeclaration;
     } else {
         /* D3D9 declaration */
-        vertexDeclaration = (IWineD3DVertexDeclarationImpl *)((IWineD3DDeviceImpl *)This->wineD3DDevice)->stateBlock->vertexDecl;
+        vertexDeclaration = (IWineD3DVertexDeclarationImpl *) (deviceImpl->stateBlock->vertexDecl);
     }
 
     if (vertexDeclaration) {
@@ -1139,8 +1141,8 @@ static HRESULT WINAPI IWineD3DVertexShad
 
 static HRESULT WINAPI IWineD3DVertexShaderImpl_GetDevice(IWineD3DVertexShader* iface, IWineD3DDevice **pDevice){
     IWineD3DVertexShaderImpl *This = (IWineD3DVertexShaderImpl *)iface;
-    IWineD3DDevice_AddRef((IWineD3DDevice *)This->wineD3DDevice);
-    *pDevice = (IWineD3DDevice *)This->wineD3DDevice;
+    IWineD3DDevice_AddRef(This->baseShader.device);
+    *pDevice = This->baseShader.device;
     TRACE("(%p) returning %p\n", This, *pDevice);
     return WINED3D_OK;
 }
@@ -1177,6 +1179,7 @@ static HRESULT WINAPI IWineD3DVertexShad
 static HRESULT WINAPI IWineD3DVertexShaderImpl_SetFunction(IWineD3DVertexShader *iface, CONST DWORD *pFunction) {
 
     IWineD3DVertexShaderImpl *This =(IWineD3DVertexShaderImpl *)iface;
+    IWineD3DDeviceImpl *deviceImpl = (IWineD3DDeviceImpl *) This->baseShader.device;
     HRESULT hr;
     shader_reg_maps *reg_maps = &This->baseShader.reg_maps;
 
@@ -1204,7 +1207,7 @@ static HRESULT WINAPI IWineD3DVertexShad
     /* Second pass: figure out registers used, semantics, etc.. */
     memset(reg_maps, 0, sizeof(shader_reg_maps));
     hr = shader_get_registers_used((IWineD3DBaseShader*) This, reg_maps,
-       This->semantics_in, This->semantics_out, pFunction, This->wineD3DDevice->stateBlock);
+       This->semantics_in, This->semantics_out, pFunction, deviceImpl->stateBlock);
     if (hr != WINED3D_OK) return hr;
 
     This->baseShader.shader_mode = wined3d_settings.vs_selected_mode;
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index 9b36a70..99731de 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -5,6 +5,7 @@
  * Copyright 2002-2003 Raphael Junqueira
  * Copyright 2004 Jason Edmeades
  * Copyright 2005 Oliver Stieber
+ * Copyright 2006 Ivan Gyurdiev
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1405,7 +1406,9 @@ typedef struct SHADER_OPCODE {
 } SHADER_OPCODE;
 
 typedef struct SHADER_OPCODE_ARG {
+    IWineD3DDevice* device;
     IWineD3DBaseShader* shader;
+    IWineD3DStateBlock* stateBlock;
     shader_reg_maps* reg_maps;
     CONST SHADER_OPCODE* opcode;
     DWORD opcode_token;
@@ -1583,6 +1586,9 @@ typedef struct IWineD3DBaseShaderClass
     struct list constantsI;
     shader_reg_maps reg_maps;
 
+    /* Pointer to the parent device */
+    IWineD3DDevice *device;
+
 } IWineD3DBaseShaderClass;
 
 typedef struct IWineD3DBaseShaderImpl {
@@ -1679,7 +1685,6 @@ typedef struct IWineD3DVertexShaderImpl 
 
     /* IWineD3DVertexShaderImpl */
     IUnknown                    *parent;
-    IWineD3DDeviceImpl          *wineD3DDevice;
 
     char                        usesFog;
     DWORD                       usage;
@@ -1713,7 +1718,6 @@ typedef struct IWineD3DPixelShaderImpl {
 
     /* IWineD3DPixelShaderImpl */
     IUnknown                   *parent;
-    IWineD3DDeviceImpl         *wineD3DDevice;
 
     /* Pixel shader input semantics */
     semantic semantics_in [MAX_REG_INPUT];
-- 
1.4.1



More information about the wine-patches mailing list