Bug 35852 - [bisected pineview] oglc case pxconv-read failed
Summary: [bisected pineview] oglc case pxconv-read failed
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: All Linux (All)
: high normal
Assignee: Marek Olšák
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-31 20:07 UTC by fangxun
Modified: 2011-05-05 03:37 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments
workaround (994 bytes, patch)
2011-04-18 13:56 UTC, Marek Olšák 		Details | Splinter Review
View All

Description fangxun 2011-03-31 20:07:16 UTC 
System Environment:
--------------------------
Arch:           i386
Platform:       Pineview
Libdrm: (master)2.4.24-9-g5cb554a0d6e986f2d7300a91d95983fa09b17f65
Mesa:   (master)5eb9f687087a4bc71775a32efcd848fc6cd67694
Xserver:(master)xorg-server-1.10.0-143-g327e1d88012102af6aca6c6840aa0ed3c7041a77
Xf86_video_intel:(master)2.14.902-2-g630d77bf10ba6234bb9c04538636f7d8aa319aea
Kernel: (drm-intel-next)f0c860246472248a534656d6cdbed5a36d1feb2e


Bug detailed description:
-------------------------
This is regression. There are other three cases failed due to the same cause: pxstore-read, pxtrans-read, pbo subcase texImage.1PBOChangeInternalFormat.
Bisect shows e5c6a92a12b5cd7db205d72039f58d302b0be9d5 is the first bad commit.
commit e5c6a92a12b5cd7db205d72039f58d302b0be9d5
Author:     Marek Olšák <maraeo@gmail.com>
AuthorDate: Tue Feb 15 23:30:23 2011 +0100
Commit:     Marek Olšák <maraeo@gmail.com>
CommitDate: Tue Mar 29 12:04:55 2011 +0200

    mesa: implement clamping controls (ARB_color_buffer_float)

    Squashed commit of the following:

    Author: Marek Olšák <maraeo@gmail.com>

        mesa: fix getteximage so that it doesn't clamp values
        mesa: update the compute_version function
        mesa: add display list support for ARB_color_buffer_float
        mesa: fix glGet query with GL_ALPHA_TEST_REF and ARB_color_buffer_float
Comment 1 Ian Romanick 2011-04-02 17:27:01 UTC 
Each of these tests has many, many subtests.  Do we have any idea what specifically is failing?
Comment 2 Ian Romanick 2011-04-02 17:33:59 UTC 
I've also noticed that glean's readPixSanity fails on master but passes on 7.10.  Do you know if that bisects to the same commit?
Comment 3 fangxun 2011-04-05 18:24:42 UTC 
glean's readPixSanity failure is not the bisect commit caused.

Following is oglc pxconv-read subtest error info:
PixelConvertCompareRead ERROR: Test failed with format PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {2.266}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {-1.000}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {2.266, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255, 0.945}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {-1.000, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267, 0.949}. Expected {1.000, 0.949}.
Comment 4 Ian Romanick 2011-04-05 19:10:20 UTC 
(In reply to comment #3)
> glean's readPixSanity failure is not the bisect commit caused.
> 
> Following is oglc pxconv-read subtest error info:
> PixelConvertCompareRead ERROR: Test failed with format PixelConvertCompareRead
> ERROR: Test failed with format GL_LUMINANCE and type GL_HALF_FLOAT_ARB,
> generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {2.266}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255}. Expected {1.000}. 

For glReadPixels, the RGB-to-luminance conversion is (see page 225 of the OpenGL 2.1 spec):

    L=R+G+B

If the source color is (1.000, 0.341, 0.925, 0.949), that's (0x7f, 0x2b, 0x75, 0x78) as signed bytes.  Without clamping, 0x7f + 0x2b + 0x75 = 0x11f.  Masking that to a signed byte gives 0x1f, and converting that to float gives 0.244.

It looks like the post-conversion clamp isn't happening.

> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {-1.000}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {2.266, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255, 0.945}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {-1.000, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267, 0.949}. Expected {1.000, 0.949}.
Comment 5 Ian Romanick 2011-04-05 19:11:35 UTC 
I'm changing the component to "Mesa core" because the conversions happen there.  I suspect this same bug occurs with swrast.
Comment 6 Gordon Jin 2011-04-17 22:17:31 UTC 
Ian, can you follow up with this bug?
Comment 7 Marek Olšák 2011-04-18 13:56:42 UTC 
Created attachment 45783 [details] [review]
workaround

Does the attached patch fix the bug?
Comment 8 Ian Romanick 2011-04-19 18:28:04 UTC 
(In reply to comment #7)
> Created an attachment (id=45783) [details]
> workaround
> 
> Does the attached patch fix the bug?

That fixes the test case here.

It seems like the tests that cause IMAGE_CLAMP_BIT to be set for the read case should be refactored to a utility function that, as your patch suggests, can be called from the callers of _mesa_pack_rgba_span_float.
Comment 9 fangxun 2011-04-20 00:22:51 UTC 
Confirm the patch fix the bug.
Comment 10 Marek Olšák 2011-04-21 01:21:34 UTC 
Please try this patch which should be the final fix:

http://lists.freedesktop.org/archives/mesa-dev/2011-April/006910.html
Comment 11 fangxun 2011-04-21 02:27:53 UTC 
The final patch fixes the bug on pineview. 
Tested by: Fang Xun <xunx.fang@intel.com>
Comment 12 Marek Olšák 2011-04-21 21:42:55 UTC 
Fixed with 1faf079a692bbf4b24c8e83fa2b331c1e3b58e13. Closing.
Comment 13 fangxun 2011-05-05 03:37:26 UTC 
Verified with mesa master fc30910c65e7ab078b900c29d2066e45d3edd8c2.

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.