Re: wined3d: GLSL Patch feedback requested

[Raphael]

On Thursday 08 June 2006 17:35, Jason Green wrote:
> The current cumulative patch is located here:
>
> http://cmhousing.net/wine/glsl_cumul.diff

I hate you, reviewing all this patch made me crazy.

Anyway is an impressive work

I only have one  question:
 i see you have a cache to reuse prgid in set_glsl_shader_program but what 
happens if user/app change the shaders content (for example using 
SetFunction) ? your pre-compiled program will be correct ?
(because i don't see any list cleanup in shaders function reset)

And many comments:

void inline  drawPrimitiveDrawStrided(IWineD3DDevice *iface, BOOL 
useVertexShaderFunction, BOOL usePixelShaderFunction, int useHW, 
WineDirect3DVertexStridedData *dataLocations,
 UINT numberOfvertices, UINT numberOfIndicies, GLenum glPrimType, const void 
*idxData, short idxSize, int minIndex, long StartIdx) {
     IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
@@ -1791,91 +1920,59 @@ #endif

TRACE("Loaded arrays\n");

+        /* Bind the correct GLSL shader program based on the currently set 
vertex & pixel shaders. */
+        if (wined3d_settings.shader_mode == SHADER_GLSL) {
+            GLhandleARB programId;
+            
+            set_glsl_shader_program(iface);
+            programId = This->stateBlock->shaderPrgId;    
+
+            if (programId != 0) {
+                /* Start using this program ID */
+                TRACE_(d3d_shader)("Using GLSL program %u\n", programId);
+                GL_EXTCALL(glUseProgramObjectARB(programId));
+                checkGLcall("glUseProgramObjectARB");
+            } 
+        }
+        
         if (useVertexShaderFunction) {
-

- It is correct to have GLSL binding code out of "if 
(useVertexShaderFunction)" block ?

- i think moving binding code (GLSL and ARB) on a new base shader method (ex 
IWineD3DBaseShaderImpl_Bind) will be cleaner (and permit to not have GLS/ARB 
code in drawprim)

- same for shaders constant loading

Best Regards,
Raphael