Bug 55265 - [Bisected IVB SNB]Piglit spec/EXT_framebuffer_multisample/accuracy 4 srgb fails
Summary: [Bisected IVB SNB]Piglit spec/EXT_framebuffer_multisample/accuracy 4 srgb fails
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: All Linux (All)
: high major
Assignee: Paul Berry
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-24 02:04 UTC by lu hua
Modified: 2012-09-26 01:36 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description lu hua 2012-09-24 02:04:06 UTC
System Environment:
--------------------------
Arch:             x86_64
Platform:         Ivybridge
Libdrm:	(master)libdrm-2.4.39-9-g2426a6a7112ae62755408a371831eddbe2d89d99
Mesa:	(master)328961d95586931a17fe81ba816d362e8389c105
Xserver:(master)xorg-server-1.13.0-9-gd01921ec18c21f21d377b60626cc2d3418b84a7c
Xf86_video_intel:(master)2.20.8-6-gd853064e7eebc5719645c12605782f995131a6fe
Libva:	(staging)a78128ac9a52b7567296e076c3cd9e5b7ea640ad
Libva_intel_driver:(staging)eb5f7f88fbd9085c3346a6b00698cef091e2ece2
Kernel:	(drm-intel-nightly) f0db8c3e57486ae7fdbc52bc157f997394d7d11d


Bug detailed description:
-------------------------
It fails on ivybridge and sandybridge with mesa master branch.It doesn't happen on mesa 9.0 branch.

output:
Pixels that should be unlit
  count = 214644
  Perfect output
Pixels that should be totally lit
  count = 28972
  Perfect output
Pixels that should be partially lit
  count = 18528
  RMS error = 0.188770
The error threshold for this test is 0.119880
PIGLIT: {'result': 'fail' }

Bisect shows:e2249e8c4d06a85d6389ba1689e15d7e29aa4dff is the first bad commit.
commit e2249e8c4d06a85d6389ba1689e15d7e29aa4dff
Author:     Kenneth Graunke <kenneth@whitecape.org>
AuthorDate: Tue Sep 11 16:20:43 2012 -0700
Commit:     Kenneth Graunke <kenneth@whitecape.org>
CommitDate: Thu Sep 20 14:48:02 2012 -0700

    i965/blorp: Add support for blits between SRGB and linear formats.

    Fixes colorspace issues in L4D2 when multisampling is enabled (the
    scene was far too dark, but the flashlight area was way too bright).

    The nVidia and AMD binary drivers both allow this kind of blit.

    NOTE: This is a candidate for the 9.0 branch.

    Reviewed-by: Paul Berry <stereotype441@gmail.com>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>


Following cases also fail and have same bisect commit:
spec_EXT_framebuffer_multisample_accuracy_4_srgb_depthstencil
spec_EXT_framebuffer_multisample_accuracy_4_srgb_small
spec_EXT_framebuffer_multisample_accuracy_4_srgb_small_depthstencil
spec_EXT_framebuffer_multisample_accuracy_8_srgb
spec_EXT_framebuffer_multisample_accuracy_8_srgb_depthstencil
spec_EXT_framebuffer_multisample_accuracy_8_srgb_small
spec_EXT_framebuffer_multisample_accuracy_8_srgb_small_depthstencil

Reproduce steps:
----------------------------
1. xinit
2. ./bin/ext_framebuffer_multisample-accuracy 4 srgb -auto
Comment 1 Kenneth Graunke 2012-09-24 03:43:44 UTC
Ugh.  Haven't looked yet, but CC'ing Paul since he'll definitely be interested.
Comment 2 Paul Berry 2012-09-24 12:58:51 UTC
The test that's failing is verifying that when doing an MSAA resolve from an sRGB buffer to an sRGB buffer, samples are blended together in sRGB space, not linearly.  This is suggested by the following text from the spec (from GL 4.3, 17.3.12 Additional Multisample Fragment Operations):

"If the framebuffer contains sRGB values, then it is recommended that the an average of sample values is computed in a linearized space, as for blending (see section 17.3.8). Otherwise, a simple average computed independently for each color component is recommended."

The reason it's failing is that blorp's fragment program always uses a simple average to do its multisample blending, and it relies on the surface state being set up as sRGB to cause correct sRGB averaging to occur.

Commit e2249e8c4d06a85d6389ba1689e15d7e29aa4dff changed blorp to always use a non-sRGB surface state, preventing sRGB-correct averaging from occurring.

I should be able to fix this pretty quickly, so I'll take the bug.
Comment 3 Paul Berry 2012-09-24 14:25:52 UTC
A patch to fix this bug has been sent to the mesa-dev list for review (subject "i965/blorp: Fix sRGB MSAA resolves.").
Comment 4 Paul Berry 2012-09-25 00:15:38 UTC
Fixed by the following commit:

commit 124b214f094fa63ff1ddb7e9f0a1c2e0ba8214fb
Author: Paul Berry <stereotype441@gmail.com>
Date:   Mon Sep 24 05:38:32 2012 -0700

    i965/blorp: Fix sRGB MSAA resolves.
    
    Commit e2249e8c4d06a85d6389ba1689e15d7e29aa4dff (i965/blorp: Add
    support for blits between SRGB and linear formats) changed blorp to
    always configure surface states for in linear format (even if the
    underlying surface is sRGB).  This allowed sRGB-to-linear and
    linear-to-sRGB blits to occur without causing the image to be
    inappropriately brightened or darkened.
    
    However, it broke sRGB MSAA resolves, since they rely on the
    destination buffer format being sRGB in order to ensure that samples
    are averaged together in sRGB-correct fashion.
    
    This patch fixes the problem by instead configuring the source buffer
    to use the *same* format as the destination buffer.  This ensures that
    the image won't be brightened or darkened, but preserves proper sRGB
    averaging.
    
    Fixes piglit tests "EXT_framebuffer_multisample/accuracy srgb".
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55265
    
    NOTE: This is a candidate for stable release branches.
    
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Reviewed-and-tested-by: Kenneth Graunke <kenneth@whitecape.org>
Comment 5 lu hua 2012-09-26 01:36:58 UTC
Verified.Fixed by commit 124b214f094fa63ff1ddb7e9f0a1c2e0ba8214fb.
commit 124b214f094fa63ff1ddb7e9f0a1c2e0ba8214fb
Author: Paul Berry 
Date: Mon Sep 24 05:38:32 2012 -0700

i965/blorp: Fix sRGB MSAA resolves.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.