[Mesa-dev] [PATCH 0/11] Post-processing infrastructure / gsoc work

[Brian Paul]

On Tue, Aug 16, 2011 at 8:24 AM, Lauri Kasanen <cand at gmx.com> wrote:
> Hi list
>
> This patchset adds post-processing to all Gallium drivers. It's also posted to the gsoc branch at http://cgit.freedesktop.org/~cand/mesa/log/?h=gsoc if you prefer cgit.
>
> The included filters are three color ones (easy testing of chaining, should work on all drivers), two versions of MLAA, and a cel-shading filter (last three need ROUND support from the driver).
>
> The set builds on top of the three cleanup patches sent earlier; they haven't been applied to master, nor have gotten any comments.

Maybe you could re-post those?


> Current driver status w/ MLAA is that softpipe and llvmpipe work. Nouveau and r600g have bug(s), r300g does not support ROUND at the time. No idea of the other g3d drivers.
>
> Looking forward to the review comments.

How exactly does the post-processing step interface to the gallium
drivers?  I took a quick look but it didn't jump out at me.  Maybe you
could also write a documentation page to add in the docs/ directory.

Otherwise, here's some stylistic things I noted:


1. There seems to be a variety of indentation levels in different files
and the formatting for function bodies doesn't match the existing
gallium files.  For example, instead of:

int foo(args) {
   ...
}

it should be:

int
foo(args)
{
   ...
}

See docs/devinfo.html or look at other gallium sources for examples.
There's an 'indent' command you can run to fix this and other things
noted below.


2. The copyright notices reference Tungsten Graphics.  You should
probably replace that with "the authors".


3. I see quite a few cases of code like this:

   if (!pscreen) return NULL;

that should be:

   if (!pscreen)
      return NULL;

in case someone wants to put a gdb breakpoint on the return stmt.
Same thing with loop bodies, etc.


4. Where you declare an array with initialization data, put
static and const qualifiers on the declaration.  Ex:

   static const float verts[4][2][4] = { ... };


5. Please make sure each function and structure definition has a
comment explaining what it does or what it's for.  We've been using
Doxygen syntax for this elsewhere.


6. I saw a pp_umax() function that could probably be omitted; use
the MAX2() macro instead.


7. Please update the SConscript files whenever you change a Makefile.


8. We generally only use /* */-style comments in our C code.

-Brian