On 08/17/2011 01:41 AM, Lauri Kasanen wrote:
On Tue, 16 Aug 2011 18:44:35 -0600
Brian Paul<brian.e.p...@gmail.com> wrote:
Hi
Thanks for taking the time to read them through. Patch 07 seems still stuck in
the ML moderation queue.
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?
Reposted.
Looks like Corbin pushed them.
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.
The PP queue is bound to the DRI context. It is init and shut down with the
context, and called on swapbuffers (flush for dri2 hw drivers). EGL and other
such systems can implement it similarly if desired.
I don't know if this is possible, but could the post-processor be
constructed as another gallium driver that wraps other drivers? For
example, the rbug driver is a wrapper driver that intercepts most of
the context/screen methods and then passes them through to the wrapped
driver.
If the post-processor could be a wrapper, it would seem to be more
flexible and easily used with any winsys or state tracker.
Otherwise, here's some stylistic things I noted:
I was told these weren't as strict for files under their own self-contained dir
(aux/pp) vs changes to existing dirs (where I did take care to fit in).
Yeah, we were pretty lax about that with the old DRI drivers, but in
Gallium I think we'd like to be more consistent.
2. The copyright notices reference Tungsten Graphics. You should
probably replace that with "the authors".
Will change, thanks.
Will do the other changes as well.
Looks good, thanks.
I spotted one other thing. In some places you're declaring variables
after code. Ex:
struct pp_queue_t *
pp_init(struct pipe_screen *pscreen, const unsigned int *enabled)
{
pp_debug("Initializing the post-processing queue.\n");
unsigned int curpos = 0, i, tmp_req = 0;
...
Some non-gcc compilers will complain about this so it should be:
struct pp_queue_t *
pp_init(struct pipe_screen *pscreen, const unsigned int *enabled)
{
unsigned int curpos = 0, i, tmp_req = 0;
pp_debug("Initializing the post-processing queue.\n");
...
Thanks.
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
