From: Jerome G. <j.g...@gm...> - 2007-05-09 07:01:44
|
On 5/8/07, Christoph Brill <eg...@gm...> wrote: > I reviewed the cleanup done by Olliver McFadden and had the following > questions: > > -int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim) > +static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim) > > Is it necessary/usefull that the function is static? I think it's better to have static function, i am thinking of symbol export and other things like that. > > -/* Immediate implementation has been removed from CVS. */ > - > -/* vertex buffer implementation */ > - > -static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr > +static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr > > Why move all the comments to the head of the file. IMO the method should > have a doxygen comment that states it is the vertex buffer > implementation of fire_EB, right? > > > - if (num_verts > 65535) { /* not implemented yet */ > + if (num_verts > 65535) { > > Comments like this should be kept. Otherwise it looks like a hardware > limitation while the limitation can be worked around or the limitation > does not exist. > > > Last but not least is > r300_foo_bar > preferred or > r300FooBar > Which is the one mesa uses? We can use the one we like, i prefer r300_foo_bar over r300FooBar which i dislike but the choice is up to the first person who do big cleanup :) and we do not have to conform to mesa coding style for driver but use the one we like the more. best, Jerome Glisse |