On Wed, Mar 05, 2003 at 01:08:52PM -0800, Ian Romanick wrote:
Philip Brown wrote:
Also, rather than containing the struct, you could do what is done already in the drm level, with drm_map_t vs drm_local_map_t (and all over the X server code), and just extend the struct, rather than encapsulating it. So [quick-n-dirty illustration for discussion purposes]
struct dritexture{ int baseval; }; struct radeontexture { int baseval; int radeonextraval; };
So what happens when you want to refactor and move data out of radeontexture and into dritexture? Then the functions that operate on dritexture will possibly break.
how so?
If you have a non-radeon card driver using dritexture, it will ignore the extra field added. Or, it will keep using the same "base" functions, which, if you done the "refactoring' right, will be a transparent change.
If you have a radeon card driver routine that previously accessed the extra field.. it can STILL access the extra field. So what will break?
I suppose that it is doable, but it just seems wrong. Doesn't this just boil down to inheritance by conincidence? Expecting each driver to duplicate the same data structures and add their unique data onto the end, without any checking done by the compiler, seems like a bad call. If we going to do that, I would rather see it done as either a nested structure (like driTextureObject) or as a macro:
#define DRI_TEXTURE \
int baseval;struct radeontexture {
DRI_TEXTURE
int radeonextraval;
};That eliminates the worry of cut-and-paste errors. What if someone puts two of the fields from the "base" in the wrong order? The compiler won't notice it, and it would be very difficult to detect by inspection. Bugs like that are the ones that usually take (waste!) a lot of time to track down.
Also, since functions are not part of "the object", I dont see where "overriding functions" comes into play. If you dont want to use a "base" function, you just call a radeon-specific one?
That's exactly the problem! If you change from using the "base" function to using a derrived function, you have to touch the code in every single place that used the base function. If you miss one, you have a bug...a bug that may go a long time unnoticed.
Are you honestly making the argument,
"Hunting down places that called RadeonTextureTwiddle, and renaming them
to DriTextureTwiddle, is a difficult operation that can lead to bugs" ??
It should be a straightforward excercise to migrate a sub-object function, into a base-object function, as describe above. If it isnt, there are two possibilities:
a) It doesnt BELONG as a base object function. case closed. b) the writer of the sub-object function, did a poor job designing/writing the sub-object function. In which case, the author needs to be whipped^H^H^Hreeducated, and made to fix the design. Only after that is done, should it then be migrated up into the base object.
in this model, it should not be a bug magnet any more than the corresponding operation of migrating from a C++ derived class to a parent class.
I'll agree with all of that. However, I was talking about going the other way around. I meant that replacing driTextureTwiddle with radeonTextureTwiddle. In the case you were talking about radeonTextureTwiddle would presumably go away, so the compiler or the linker will let you know that something is wrong. In the other case, the code will still compile, link, and run.
You're right. Doing a global search-and-replace *should* be easy. As an experienced developer, I've seen much simpler things get botched. How many times have you send cut-and-paste style errors? It's all pretty easy stuff, but we're still human. It's nice to have a machine around to tap you on the head and say, "Hey stupid! You messed up!" :)
-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger for complex code. Debugging C/C++ programs can leave you feeling lost and disoriented. TotalView can help you find your way. Available on major UNIX and Linux platforms. Try it free. www.etnus.com
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
