[Mesa-dev] [PATCH V2 3/3] i965/blorp: Add support for single sample scaled blit with bilinear filter

[Paul Berry]

On 14 August 2013 11:46, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> On Wed, Aug 14, 2013 at 10:25 AM, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > On 9 August 2013 19:01, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> >> -   src.brw_surfaceformat = dst.brw_surfaceformat;
> >> +   if (src.num_samples > 1)
> >> +      src.brw_surfaceformat = dst.brw_surfaceformat;
> >> +   else
> >> +      dst.brw_surfaceformat = src.brw_surfaceformat;
> >
> >
> > I thought we decided that it was a bug to ever set src.brw_surfaceformat
> =
> > dst.brw_surfaceformat, and we should do dst.brw_surfaceformat =
> > src.brw_surfaceformat always.  Am I remembering wrong?
> As suggested by Ian, I modified a piglit test and checked the
> behavior on proprietary NVIDIA OpenGL 4.3 drivers. NVIDIA
> still uses destination format to decide the type of averaging
> during resolve. During our offline discussion you mentioned
> how NVIDIA's approach produce slightly better looking images
> as compared to using the source buffer format for avearging the
> samples. Also, OpenGL specification doesn't seem to be strict
> about it. So, I decided to use an if-else to take care of averaging
> format in single sample scaled blits. I don't have any strong
> opinions about it and I'm willing to post a patch to use
> dst.brw_surfaceformat = src.brw_surfaceformat for all cases.
> Please let me know if you're in favor of fixing it.
>

Ah, ok.  In that case I think you made the right decision.  Would you mind
putting a comment above the if statement documenting your reasoning?  It's
a really subtle decision and I'd hate for someone to come along in the
future and try to "fix" it without understanding the history.

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130814/d8a4120f/attachment-0001.html>