Am Donnerstag 12 April 2007 19:49 schrieb Fabian Bieler:
Remove the usesFog flag and replace it with a 'method' as suggested by Ivan Gyurdiev.
So I'm ready to start yet another design flamewar here :-)
Honestly I do not really agree with getter methods like this inside WineD3D. Yes, they do hide the implementation details, namely how the flag is stored. Yes, they do encapsulate data, like the Object Oriented Programming model says. But honestly, how much use is it to do a function call just to read a value inside wined3d?
I don't argue that we should make wined3d internals visible to outside wined3d. But inside wined3d, my personal preference is to just access the implementation structure directly. How much abstraction does such a function give us at the cost of performance? In both cases the state management code has to know an internal of the shader code, wether the shader writes to the fog coordinate. This is something only the shader should need to know, no matter how it is accessed. Why do we need it? Because the way fog works with shaders requires that either the shader sets fog params or the code setting fog params has to know about the shader. We can't change that. There are many more examples like surface sizes in the viewport and SetRenderTarget code, ...
-------------
In case of the foggy shader thing, I think we can get rid of that by just setting up the fog start and fog end when a shader is used and fog is on, no matter if the shader actually uses the fog. May even be faster in the end. Other than that, I think that the shader fog code is not quite right either, since if we need the foggy shader setup depends on the fogtablemode setting. But other simmilar cases still remain.
Stefan Dösinger wrote:
Honestly I do not really agree with getter methods like this inside WineD3D. Yes, they do hide the implementation details, namely how the flag is stored. Yes, they do encapsulate data, like the Object Oriented Programming model says. But honestly, how much use is it to do a function call just to read a value inside wined3d?
Well, your use of a redundant top-level flag does kind of remove the need for a getter - I only brought it up, since I've moved the reg_maps structure around one too many times, and that's where the first flag is stored.
I don't argue that we should make wined3d internals visible to outside wined3d. But inside wined3d, my personal preference is to just access the implementation structure directly.
The more complex and interconnected the codebase, the more it makes sense to encapsulate things. I think abstraction also benefits the less experienced developer. You make a good point that this level of detail should be internal to the shader to begin with, regardless of how it's accessed.
How much abstraction does such a function give us at the cost of performance?
You can always make the function inline (although that's also rather ugly).
Well, your use of a redundant top-level flag does kind of remove the need for a getter - I only brought it up, since I've moved the reg_maps structure around one too many times, and that's where the first flag is stored.
I agree that the top level flag was bad. It was just a redundant copy of the reg_maps content in the same form.
The more complex and interconnected the codebase, the more it makes sense to encapsulate things. I think abstraction also benefits the less experienced developer. You make a good point that this level of detail should be internal to the shader to begin with, regardless of how it's accessed.
I think for this special case its the best to see if the state management really has to know about this flag and remove it if appropiate.
Your point that we should use encapsulation to make things easier is good, but I do not think that encapsulation always makes does that. Like in the fog thing, I guess the main question for someone who does not know the code will be why the state management has to know wether the shader writes out fog. The way the state management gets the flag is not that important. But that example is not of much use anyway.
A place where we desperately need more encapsulation is the surface code. The dirtification, downloading and uploading could use some cleanup to replace all the mad flag setting and clearing.
A place where I think encapsulation wouldn't be useful is accessing the device and stateblocks. A lot of things depend on the stateblock, like Vertex Buffer loading, Shader compilation and (a bit) resource management. But in all those places the stateblock is only read(except in Resource::Release, which should actually go) and IMHO just accessing the stateblock to compare the pointers is easier to read than a Get*(), compare, check against NULL, release.
How much abstraction does such a function give us at the cost of performance?
You can always make the function inline (although that's also rather ugly).
Nah, and it is not reliable either.
-
Ivan Gyurdiev -
Stefan Dösinger