[1/7] [wined3d] add ps_np2fixup_t struct to ps_compiled_shader

Tobias Jakobi liquid.acid at gmx.net
Wed Apr 22 10:18:53 CDT 2009


> - I'm sure I mentioned this before, but please don't add code without
> using it.
I'll merge it with one of the other patches then.

> - Why is this in struct ps_compiled_shader? This really looks like
> something that should be internal to the backend.
No, that's definitely not the case.
The struct's data is used both in GLSL and ARB mode (I'll submit the ARB
patches later) - so it makes sense to put it into ps_compiled_shader.

For ARB mode this is essential since something like the prog_link struct
from GLSL doesn't exist there.
The original source of this struct is my port of the fixup code to ARB,
but introducing the constant packing for GLSL makes more data necessary
- data that should be shared.

I can post the next patchset for review if you don't believe me ;)

> - What is the point of const_cache?
Reloading the np2fixup constants.

Before I was declaring vec2 uniforms to store the tex dimensions. Stefan
found out that the compilers are inefficent and map a vec2 to a vec4.

So now I'm using vec4s only, but one vec4 is used for two textures.
So x and y store width and height of texture i, and z and w store the
width and height of texture j.

In load_np2fixup_constants I have to use glUniform4fvARB now to set the
values of the uniform. I can't simply create a float[4], zero it, set
the components that belong to the currently selected texture and upload
it, since it might overwrite a texture dimension pair that was
previously uploaded.

However while writing this, I have another idea to solve this without
using the cache.

> - Writing "GLfloat* const_cache;" is legal C, but really doesn't make
> a lot of sense. The * is part of the declarator rather than the base
> type. An example like "int* p, q;" should make this more obvious.
Your example is bad coding practice IMHO - i would never declare
variables with different types on the same line.

So your point is that I should write
GLfloat *const_cache;
Well, at least from my point of view that's purely a matter of taste. :)

> - swz is redundant. "prog->np2Fixup_data->swz & (1 << i)" really isn't
> more efficient than "idx & 1"
As far as I can see that doesn't do the same.



More information about the wine-devel mailing list