[Mesa-dev] [PATCH 2/2] i965/blorp: Add support for blits between SRGB and linear formats.

Thu Sep 20 14:05:38 PDT 2012

On 18 September 2012 14:32, Paul Berry <stereotype441 at gmail.com> wrote:

> On 17 September 2012 10:53, Paul Berry <stereotype441 at gmail.com> wrote:
>
>> On 17 September 2012 10:25, Ian Romanick <idr at freedesktop.org> wrote:
>>
>>> On 09/14/2012 02:56 AM, Paul Berry wrote:
>>>
>>>> On 11 September 2012 16:24, Kenneth Graunke <kenneth at whitecape.org
>>>> <mailto:kenneth at whitecape.org>**> wrote:
>>>>
>>>>     Fixes colorspace issues in L4D2 when multisampling is enabled (the
>>>>     scene was far too dark, but the flashlight area was way too bright).
>>>>
>>>>     NOTE: This is a candidate for the 9.0 branch.
>>>>
>>>>     Cc: Paul Berry <stereotype441 at gmail.com
>>>>     <mailto:stereotype441 at gmail.**com <stereotype441 at gmail.com>>>
>>>>     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>>>>     <mailto:kenneth at whitecape.org>**>
>>>>
>>>>
>>>>
>>>> If I'm reading the spec correctly, it's awfully contradictory.  From GL
>>>> 4.3 compatibility profile, section 18.3.1 Blitting Pixel Rectangles:
>>>>
>>>>      (1) When values are taken from the read buffer, if the value of
>>>> FRAMEBUFFER_ATTACHMENT_COLOR_**ENCODING for the framebuffer attachment
>>>> corresponding to the read buffer is SRGB (see section 9.2.3), the red,
>>>> green, and blue components are converted from the non-linear sRGB color
>>>> space according to equation 8.14.
>>>>
>>>>      (2) When values are taken from the read buffer, no linearization is
>>>> performed even if the format of the buffer is SRGB.
>>>>
>>>>      (3) When values are written to the draw buffers, blit operations
>>>> bypass most of the fragment pipeline. The only fragment operations which
>>>> affect a blit are the pixel ownership test, the scissor test, and sRGB
>>>> conversion (see section 17.3.9). Color, depth, and stencil masks (see
>>>> section 17.4.2) are ignored.
>>>>
>>>
>>> A bunch of these language changes are new, and there seem to be some
>>> issues.  Basically, sRGB and blits have been horribly broken since the
>>> beginning, and fixing it to be sensible is breaking apps.  Wine, for
>>> example, is pretty pissed about this.  Note that *none* of the
>>> closed-source drivers implement the behavior from the spec.  *None*.
>>
>>
>> Do you know what behaviour is typically implemented or should we go out
>> and do some tests?
>>
>
> I ran some tests on my system with the nVidia linux binary driver, and
> here's what I found:
>
> - Formats GL_RGBA and GL_SRGB8_ALPHA8 are considered "identical" formats
> for the purpose of seeing whether an MSAA blit is permitted.
> - glBlitFramebuffer() always performs a direct copy (it neither converts
> to nor from sRGB space, regardless of the setting of FRAMEBUFFER_SRGB and
> regardless of the formats of the source and destination framebuffers).
>

Ok, Ken and I just tried the test on an ATI system, and here's what we
found:

- On his ATI system, you can't create sRGB buffers using
glRenderbufferStorage() or glRenderbufferStorageMultisample() (you get a
GL_INVALID_ENUM error, and then later the driver crashes).  This appears to
be a driver bug.  Fortunately However you *can* create sRGB textures and
attach them to framebuffers.  So we modified the test to create the
single-sampled buffers using textures, create the linear MSAA buffer using
glRenderbufferStorageMultisample(), and skip the sRGB MSAA buffer.
- With these changes, the ATI system behaves the same as the nVidia system.

That's enough to convince me.  Ken, you can consider both of your patches:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>

I'll take responsibility for updating my test with pass/fail criteria (I'll
also update the test to test both textures and renderbuffers, since Mesa
blits use different code paths for those two possibilities).  And I'll make
any remaining Mesa fixes that are necessary to get the test to pass.

>
> The test I performed is availble in branch blit-test of git://
> github.com/stereotype441/piglit.git.  Simply run
> "arb_framebuffer_srgb-blit -auto" and collect standard output.  I'll attach
> the output I got on my nVidia system to the end of this email.  Note: at
> the moment the test simply outputs a description of how the implementation
> behaves--it doesn't do any pass/fail testing.  I'll happily add pass/fail
> criteria once we decide what behaviour is "correct".
>
> Considering Ian's comments about sRGB and blits having been "horibly
> broken since the beginning", I think what the nVidia driver is doing seems
> pretty reasonable (it's simple, it's likely to do what the client
> application wants in most cases, and it's consistent with the connotation
> of "blit" being a block copy with no image interpretation).  Also, it
> wouldn't be too hard to alter the spec to make it match the nVidia driver's
> behaviour.  All we would need to do is remove paragraph (1), drop the text
> "and sRGB conversion (see section 17.3.9)" from paragraph (3), and alter
> paragraph (4) to note that formats that differ only by whether or not they
> are sRGB are considered "identical" for purposes of determining whether
> multisampled blits are allowed.
>
> I'm interested in hearing if other implementations do the same.  Can
> someone (Ken or Anuj perhaps) run my piglit test on an AMD platform and
> send out the results?
>
> Assuming we don't see any surprises when we test on AMD, and we decide
> that the nVidia behaviour makes sense, then I think Ken's patches are
> good--they make Mesa/i965 behave like nVidia in all cases but three:
>
> - scaled blit winsys -> srgb
> - scaled blit linear -> srgb
> - scaled blit srgb -> srgb
>
> And these three seem to be broken due to a bug in Meta: it only disables
> sRGB encoding when blitting with the blitframebuffer_texture() approach.
> If it has to fall back to copying the source rectangle into a temporary
> texture (which it does when blitting between renderbuffers), then it fails
> to disable sRGB encoding.  That seems easily fixable.
>
> Output of "arb_framebuffer_srgb-blit -auto" on my nVidia system follows:
>
> -------------------------
> Testing readback in winsys fbo: Direct copy (error=0.000000)
> Testing readback in linear fbo: Direct copy (error=0.000000)
> Testing readback in srgb fbo: Direct copy (error=0.000000)
> Testing readback in linear MSAA fbo: Direct copy (error=0.000000)
> Testing readback in srgb MSAA fbo: Direct copy (error=0.000000)
> Testing 1:1 blit winsys -> winsys (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> winsys (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit winsys -> winsys (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> winsys (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit winsys -> linear (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> linear (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit winsys -> linear (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> linear (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit winsys -> srgb (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> srgb (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit winsys -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit winsys -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit winsys -> linear MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit winsys -> linear MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit winsys -> linear MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit winsys -> linear MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit winsys -> srgb MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit winsys -> srgb MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit winsys -> srgb MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit winsys -> srgb MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear -> winsys (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> winsys (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit linear -> winsys (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> winsys (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit linear -> linear (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> linear (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit linear -> linear (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> linear (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit linear -> srgb (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> srgb (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit linear -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit linear -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit linear -> linear MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear -> linear MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit linear -> linear MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear -> linear MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear -> srgb MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear -> srgb MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit linear -> srgb MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear -> srgb MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb -> winsys (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> winsys (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit srgb -> winsys (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> winsys (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit srgb -> linear (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> linear (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing 1:1 blit srgb -> linear (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> linear (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit srgb -> srgb (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> srgb (FRAMEBUFFER_SRGB disabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit srgb -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing 1:1 blit srgb -> linear MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb -> linear MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb -> linear MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb -> linear MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb -> srgb MSAA (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb -> srgb MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb -> srgb MSAA (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb -> srgb MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> winsys (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> winsys (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> winsys (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> winsys (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> linear (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> linear (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> linear (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> linear (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> srgb (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> srgb (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> srgb (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit linear MSAA -> srgb (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit linear MSAA -> linear MSAA (FRAMEBUFFER_SRGB disabled):
> Direct copy (error=0.000000)
> Testing scaled blit linear MSAA -> linear MSAA (FRAMEBUFFER_SRGB
> disabled): No action (error=0.000000)
> Testing 1:1 blit linear MSAA -> linear MSAA (FRAMEBUFFER_SRGB enabled):
> Direct copy (error=0.000000)
> Testing scaled blit linear MSAA -> linear MSAA (FRAMEBUFFER_SRGB enabled):
> No action (error=0.000000)
> Testing 1:1 blit linear MSAA -> srgb MSAA (FRAMEBUFFER_SRGB disabled):
> Direct copy (error=0.000000)
> Testing scaled blit linear MSAA -> srgb MSAA (FRAMEBUFFER_SRGB disabled):
> No action (error=0.000000)
> Testing 1:1 blit linear MSAA -> srgb MSAA (FRAMEBUFFER_SRGB enabled):
> Direct copy (error=0.000000)
> Testing scaled blit linear MSAA -> srgb MSAA (FRAMEBUFFER_SRGB enabled):
> No action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> winsys (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> winsys (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> winsys (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> winsys (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> linear (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> linear (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> linear (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> linear (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> srgb (FRAMEBUFFER_SRGB disabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> srgb (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> srgb (FRAMEBUFFER_SRGB enabled): Direct copy
> (error=0.000000)
> Testing scaled blit srgb MSAA -> srgb (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> linear MSAA (FRAMEBUFFER_SRGB disabled):
> Direct copy (error=0.000000)
> Testing scaled blit srgb MSAA -> linear MSAA (FRAMEBUFFER_SRGB disabled):
> No action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> linear MSAA (FRAMEBUFFER_SRGB enabled):
> Direct copy (error=0.000000)
> Testing scaled blit srgb MSAA -> linear MSAA (FRAMEBUFFER_SRGB enabled):
> No action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> srgb MSAA (FRAMEBUFFER_SRGB disabled):
> Direct copy (error=0.000000)
> Testing scaled blit srgb MSAA -> srgb MSAA (FRAMEBUFFER_SRGB disabled): No
> action (error=0.000000)
> Testing 1:1 blit srgb MSAA -> srgb MSAA (FRAMEBUFFER_SRGB enabled): Direct
> copy (error=0.000000)
> Testing scaled blit srgb MSAA -> srgb MSAA (FRAMEBUFFER_SRGB enabled): No
> action (error=0.000000)
> PIGLIT: {'result': 'pass' }
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120920/05491d3a/attachment-0001.html>