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

Henri Verbeet hverbeet at gmail.com
Wed Apr 22 11:39:40 CDT 2009


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.

>> - 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.

>> - 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).



More information about the wine-devel mailing list