From: Brian P. <bri...@tu...> - 2007-05-09 14:10:08
|
Jerome Glisse wrote: > 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. Yes, make functions static whenever possible. >> -/* 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. Yes, core Mesa has a fairly consistant naming scheme but it's the prerogative of the driver writers to choose their style. That said, naming within each driver should be consistant. -Brian |