2009/4/22 Tobias Jakobi <liquid.a...@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).