http://bugs.winehq.org/show_bug.cgi?id=14762
--- Comment #45 from Stefan Dösinger stefandoesinger@gmx.at 2008-12-04 19:20:58 --- I forgot to add a few things:
The patch looks reasonable, appart of the things noted above here are a few tiny recommendations:
+ for(i = 0; i < MAX_FRAGMENT_SAMPLERS; ++i) { + if(compile_args.texrect_fixup & (1 << i)) { + sprintf(name, "PsamplerRectFixup%d", i); + entry->rectFixup_location[i] = GL_EXTCALL(glGetUniformLocationARB(programId, name)); + } else + entry->rectFixup_location[i] = -1;
This is not needed, glGetUniformLocationARB returns -1 if the name isn't found. So GL does this check for you. I personally have some problems if havin {} in one of case and not in the other, but that's mostly my personal opinion. When I looked at the code I thought that a } is missing there
+ if(GL_TEXTURE_RECTANGLE_ARB == IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler])) + args->texrect_fixup |= (1 << sampler); The line above that adds trailing whitespace
+ /* trigger shader constant reloading (for texture coordinate fixup, when RECT samplers are used) */ + if(GL_TEXTURE_RECTANGLE_ARB == IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) && I think, but I haven't checked, that the code around this place also uses GetTextureDimensions in a few cases. It may be an idea to read it once and store it in a local variable to avoid calling the COM method over and over.
Also, my unfinished sentence: If you really need a surface in the glsl_shader.c code, you have to use IWineD3DTexture_GetSurfaceLevel or IWineD33DBaseTextureImpl::surfaces[level]. I don't think you need it though
Your patch also ignores one case: When a vertex shader is used(texcoord mul doesn't apply), but no pixel shader is used(your new code doesn't apply). In this case the texrect will still be broken, but I think it is better to ignore this for now. It is broken now, it will be broken after your patch, so things don't get worse, but your patch fixes the much more common case of vertex+pixel shader used.