[4/4] [wined3d] trigger constant reloading when RECT textures change

[Stefan Dösinger]

Am Mittwoch, 25. März 2009 14:52:15 schrieb Tobias Jakobi:

A few comments on all the patches. I am a bit busy at university right now, so 
I didn't have a chance to answer earlier:

From patch 4/4:
> +        if (!use_ps(stateblock)) {
> +            TRACE("Non power two matrix multiply fixup\n");
> +            glMultMatrixf(((IWineD3DTextureImpl *) 
stateblock->textures[texUnit])->baseTexture.pow2Matrix);
> +        }
I think a comment would be helpful why this isn't done when pixel shaders are 
used, as this can't be seen from the context of this code.

> +        if (GL_TEXTURE_RECTANGLE_ARB == 
IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) &&
> +            use_ps(stateblock)) {
Maybe it would be helpful to add a flag that says if the fixup matrix is 
identity. WineD3D can emulate np2 textures with padded GL_TEXTURE_2D textures 
as well. Now I can't think of any hardware that has shaders, but not 
GL_ARB_texture_rectangle, but checking for GL_TEXTURE_RECTANGLE_ARB to find 
out if we need a fixup isn't that nice(We might do it in other places too, 
though)

From patch 1:
> +    /* Bitmap for texture rect coord fixups (32 samplers max currently).
> +     * Increase size when MAX_FRAGMENT_SAMPLERS grows. */
> +    UINT                        texrect_fixup;
I think there's no point in supporting more than 16 samplers. That's the limit 
in d3d9 and will never change, and d3d10 makes unconditional NP2 support 
mandatory. The ps_compile_args structure is memcmp'ed every time the shader 
is changed, so the size matters. I also recommend to move it to the bottom of 
the structure, since it will be 0x0000 in most cases, so it prolongs the 
number of bytes the memcmp has to check before it finds a difference.(ok: In 
which direction does memcmp search?)

General:
I also think that you could merge the 4 patches into one or two. I appreciate 
the effort to make the patches small, but that shouldn't be a self-sufficient 
purpose. Patches 1 and 2 can certainly be merged, as patch 1 alone doesn't do 
anything. Patch 4 could be merged into patch 3 because it is required for the 
code in patch 3 to work correctly.

The patches leave out one corner case(Subject for a separate patch though): 
The use of a vertex shader with fixed function fragment processing. In this 
case the vertex shader has to take care of the fixup. In GLSL that can 
conveniently be done in the pixelshader<->vertexshader glue function. It 
could read the same uniform and multiply the written texture coordinate.

Once all the cases work in GLSL, we could experiment with the R and RG float 
formats from GL_NV_float_buffer, since they're supported on RECT textures 
only, but may come handy on pre-geforce 8 hardware which doesn't support the 
ARB_RG extension.