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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
Url : http://www.winehq.org/pipermail/wine-devel/attachments/20060613/101e9566/attachment.pgp