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

[Tobias Jakobi]

Henri Verbeet wrote:
> 2009/4/22 Tobias Jakobi <liquid.acid at gmx.net>:
>>> - 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.
>>
> Sure, but it's still internal bookkeeping of the backends. Everything
> the backend needs from the outside world is the np2_fixup field from
> struct ps_compile_args.
> 
>> For ARB mode this is essential since something like the prog_link struct
>> from GLSL doesn't exist there.
> It's not terribly hard to introduce something like that. That could
> mean adding a handle table to the ARB backend, or expanding prgId to
> be a pointer, but that's ok.
So, is that a suggestion? I don't think that's for me to decide.

You should make clear if I should implement it this way or use another
approach. From my point of view it makes sense to share the data, so
putting this into ps_compiled_shader is my currently best option.

But if you say, that the current approach is a no-go - I'll try the
pointer-approach.

So, how should I do it?


> 
>>> - 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.
>>
> The point is that writing "GLfloat* const_cache;" implies the * is
> part of the type instead of the declarator. C doesn't work that way,
> it's misleading at best.
IMHO that's splitting hairs.

You could apply the same this for pointer casting, forbidding something
like
var = (TYPE*)var2;
and instead enforce
var = (TYPE *)var2;

However, const_cache is now gone - so this shouldn't be a problem anymore ;)


> 
>>> - 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.
>>
> Not in the current code, no. But if you store the index of the fixup
> parameter instead of the index of the packed uniform the parameter is
> stored in, you can derive the index of the packed uniform as (idx >>
> 1) and the position within that uniform as (idx & 1).
> 
I don't think I can follow you.

I tried some similar approach in the beginning, but it I think it failed
if tex1 and tex3 need fixup, but tex2 does not.
The approach didn't like "holes" in the indices. However we can't assume
that all texture samples that need fixup are consecutive.