[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #45 from Stefan Dösinger <stefandoesinger at 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.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.