Re: wined3d: Fix regression from dynamic constants patch
There were plenty of ways to fix this. One would have been to unroll all of the memset/memcpy calls into functions that explicitly set every value on the stateblock. While this might have been more "correct", it would be harder to maintain.
I argue that an explicit initializer is always better than an implicit one. An implicit initializer is basically programmer laziness imho (I consider myself very lazy :) When you add a new feature, you should check its initialization one way or the other - can't rely on calloc assumptions, the code has to be verified. So, while you're doing that, it's easy to add an initializer.
Plus, we might lose some performance from numerous sets vs. a single memset routine. Code correctness, clarity and reliability is much more important than any performance considerations imho. Saving 1 cpu cycle here or there does not matter in the long run. So, I chose to make the number of constants more fixed again (less dynamic). The number of available constants is now capped at 256, which is the minimum required number for shader model 3.0. Software shaders require 8192 constants. They're not supported right now, but that gives you an idea of how many we need to support.
Anyway, I'm not sure which way is better - maybe you're right that making things dynamic is not worth the trouble. However, I think that shows lack of flexibility in the stateblock, and if we ever need to make other things dynamic in the future we might run into the same issue.
The app will use the min() of 256 and the number reported by the card So it uses the lower value? How come?
I was getting ready to argue with you this point, but you made a comment on IRC that changed my mind: "I thought the idea was to support the full range of the hardware". And, you're right. :-) So, please ignore this patch for now - I'll re-work it over the weekend and resubmit. On 7/21/06, Ivan Gyurdiev <ivg231(a)gmail.com> wrote:
There were plenty of ways to fix this. One would have been to unroll all of the memset/memcpy calls into functions that explicitly set every value on the stateblock. While this might have been more "correct", it would be harder to maintain.
I argue that an explicit initializer is always better than an implicit one. An implicit initializer is basically programmer laziness imho (I consider myself very lazy :) When you add a new feature, you should check its initialization one way or the other - can't rely on calloc assumptions, the code has to be verified. So, while you're doing that, it's easy to add an initializer.
Plus, we might lose some performance from numerous sets vs. a single memset routine. Code correctness, clarity and reliability is much more important than any performance considerations imho. Saving 1 cpu cycle here or there does not matter in the long run. So, I chose to make the number of constants more fixed again (less dynamic). The number of available constants is now capped at 256, which is the minimum required number for shader model 3.0. Software shaders require 8192 constants. They're not supported right now, but that gives you an idea of how many we need to support.
Anyway, I'm not sure which way is better - maybe you're right that making things dynamic is not worth the trouble. However, I think that shows lack of flexibility in the stateblock, and if we ever need to make other things dynamic in the future we might run into the same issue.
The app will use the min() of 256 and the number reported by the card So it uses the lower value? How come?
participants (2)
-
Ivan Gyurdiev -
Jason Green