2009/6/15 Tobias Jakobi Liquid.Acid@gmx.net:
Why do you need this to be in a separate loop? Also, could you please fix your mailer to set a proper Content-Type, instead of application/octet-stream?
Hi Henri,
I do this for patch 3/4, otherwise the code gets kinda ugly.
A good point probably is that the code is indeed rarely used, since it was written specifically for the FX generation. So it makes sense to skip it early -> put it into a seperate loop and "conceal" it with an if-statement. Like Stefan stated on the FDO bugtracker: "which can be disabled fairly effectively at runtime if not needed" (referring to the np2fixup code) I want it to stay this way (which includes mostly not interfering with the rest of the code IF that's possible).
However the main point is patch 3/4, where a seperate loop makes the code a lot neater - which I think is a good thing to have :)
Greets, Tobias
Am Dienstag, 16. Juni 2009 10:34:52 schrieb Tobias Jakobi:
Hi Henri,
I do this for patch 3/4, otherwise the code gets kinda ugly.
Like Stefan stated on the FDO bugtracker: "which can be disabled fairly effectively at runtime if not needed" (referring to the np2fixup code)
Actually that comment was about the state change trouble NP2 fixups cause - since we can ignore the additional state handlers needed to adjust the TF2 matrices and call the NP2 constant load code, the NP2 code doesn't add much burden if its not used.
I for one am not really worrying about one loop more or less in shader_generate_glsl_declarations. That code isn't part of the general rendering loop, its only called once per compiled GL shader. Yes, I should still be worrying about it - long GL shader compile times cause real problems at the moment - but one if condition more or less doesn't hurt, if it keeps the code cleaner.
That said, I am personally fine with keeping that in a separate loop. I don't object to not doing that though.
2009/6/16 Stefan Dösinger stefandoesinger@gmx.at:
at the moment - but one if condition more or less doesn't hurt, if it keeps the code cleaner.
I'm not convinced it does.
Actually that comment was about the state change trouble NP2 fixups cause
since we can ignore the additional state handlers needed to adjust the TF2 matrices and call the NP2 constant load code, the NP2 code doesn't add much burden if its not used.
OK, I see. But the same reason should apply to generating shader code.
I for one am not really worrying about one loop more or less in shader_generate_glsl_declarations. That code isn't part of the general rendering loop, its only called once per compiled GL shader. Yes, I should still be worrying about it - long GL shader compile times cause real problems at the moment - but one if condition more or less doesn't hurt, if it keeps the code cleaner.
Erm, I think I have to make myself even more clear here. Moving the code inside a seperate loop is actually REDUCING the amount of branching.
Currently for each 2D texture we do this check here: if (pshader && ps_args->np2_fixup & (1 << i))
Perhaps with 99% of the hardware/driver configs wine never actually uses the np2fixup code.
With the seperate loop we reduce this to a single branch, which will mostly just skip the code. In the unusual case that the hardware/driver needs the fixup this adds one more branch. Again THIS is the unlikely case.
Another thing: The old code just created a vec2 for each texture that needed fixup. That integrated quite well with the already existing loop that created the sampler uniforms. This is no longer true with the new code.
To appease Henri I'm going to merge the first patch into patch 3. ;)
Greets, Tobias
Am Dienstag, 16. Juni 2009 12:38:23 schrieb Tobias Jakobi:
Erm, I think I have to make myself even more clear here. Moving the code inside a seperate loop is actually REDUCING the amount of branching.
Yes - I've expressed myself badly, so you both understood it the wrong way. What I wanted to say was, if it keeps the code cleaner *not* to split the loops, its ok here. I am pretty sure those additional branches in this code cannot be measured by any benchmark.
That said, IMHO this issue is pretty academic, and I'm fine with both ways. I'll leave this particular question up to you guys.
2009/6/16 Tobias Jakobi liquid.acid@gmx.net:
However the main point is patch 3/4, where a seperate loop makes the code a lot neater - which I think is a good thing to have :)
It should probably be part of that patch then, although it's not clear to me why having those two lines in there would make things much worse.
-
Henri Verbeet -
Stefan Dösinger -
Tobias Jakobi