Bug 93338

Summary:	[BXT, SKL] piglit.spec.ext_packed_float.multisample-fast-clear gl_ext_packed_float
Product:	Mesa	Reporter:	Mark Janes <mark.a.janes>
Component:	Drivers/DRI/i965	Assignee:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	low	CC:	mark.a.janes, nroberts
Version:	git	Keywords:	regression
Hardware:	x86-64 (AMD64)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:

Description Mark Janes 2015-12-10 20:14:16 UTC

d04612b60d98ff785646affaffc3d7243deecb74 regressed this on bxt/skl

Author:     Jordan Justen <jordan.l.justen@intel.com>
AuthorDate: Wed Sep 2 15:47:33 2015 -0700
Commit:     Jordan Justen <jordan.l.justen@intel.com>
CommitDate: Wed Dec 9 23:50:38 2015 -0800

    i965: Enable ARB_compute_shader extension on supported hardware
    
    Enable ARB_compute_shader on gen7+, on hardware that supports the
    OpenGL 4.3 requirements of a local group size of 1024.
    
    With SIMD16 support, this is limited to Ivy Bridge and Haswell.
    
    Broadwell will work with a local group size up to 896 on SIMD16
    meaning programs that use this size or lower should run when setting
    MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader.
    
    Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
    Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
    Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>

/tmp/build_root/m64/lib/piglit/bin/ext_framebuffer_multisample-fast-clear GL_EXT_packed_float -auto -fbo
Using test set: GL_EXT_packed_float
Testing GL_R11F_G11F_B10F_EXT
Probe color at (0,0)
  Expected: 0.000000 0.000000 0.250000 1.000000
  Observed: -2.000000 0.000000 0.250000 1.000000
Probe color at (0,0)
  Expected: 0.000000 0.000000 0.250000 1.000000
  Observed: -0.500000 0.000000 0.250000 1.000000

Comment 1 Mark Janes 2015-12-10 20:28:45 UTC

I wrote up this bug with flawed information.  This regression appears to have been caused by the series following Jordan's patch, ending with:

ba67739b662eaf68f7a1117005e20079b2653044
References: master origin/master origin/HEAD
Author:     Neil Roberts <neil@linux.intel.com>
AuthorDate: Thu Nov 19 15:29:19 2015 +0100
Commit:     Neil Roberts <neil@linux.intel.com>
CommitDate: Thu Dec 10 11:03:58 2015 +0000

    blit: Don't take into account the Mesa format when checking MSRT blit
    
    According to the GLES3 spec, blitting between multisample FBOs with
    different internal formats should not be allowed. The
    compatible_resolve_formats function implements this check. Previously
    it had a shortcut where if the Mesa formats of the two renderbuffers
    were the same then it would assume the blit is ok. However some
    drivers map different internal formats to the same Mesa format, for
    example it might implement both GL_RGB and GL_RGBA textures with
    MESA_FORMAT_R8G8B8A_UNORM. The function is used to generate a GL error
    according to what the GL spec requires so the blit should not be
    allowed in that case. This patch just removes the shortcut so that it
    only ever looks at the internal format.
    
    Note that I posted a related patch to disable this check altogether
    for desktop GL. However this function is still used on GLES3 because
    there are conformance tests that require this behaviour so this patch
    is still useful.
    
    Cc: Marek Olšák <maraeo@gmail.com>
    Reviewed-by: Marek Olšák <marek.olsak@amd.com>

Comment 2 Neil Roberts 2015-12-11 16:46:40 UTC

Thanks for the bug report. I was aware of this but I was reluctant to fix it because the test is catching a fairly ridiculous corner case and I didn't feel good about adding a special case for it. I should have mentioned this to you when I pushed the patches, sorry.

I think I've thought of a nicer way to handle this without a special case so I've posted a patch here:

http://patchwork.freedesktop.org/patch/67912/

Comment 3 Mark Janes 2016-05-28 00:06:12 UTC

Neil is gone, we need to decide what to do with this bug.

Comment 4 Mark Janes 2016-06-22 15:49:03 UTC

11.2 is shipping with the bisected commit, so it shouldn't block the 12.0 release.

Comment 5 Neil Roberts 2018-01-11 10:19:11 UTC

This appears to have been fixed by adding a special case for it in the following commit:

https://cgit.freedesktop.org/mesa/mesa/commit/?id=f1fa4be871e13c68b50685aaf64

I think it’s kind of a shame to take this approach rather than my patch which reduced the code duplication a fair bit. But if it works it works.

Comment 6 Neil Roberts 2018-01-11 13:20:59 UTC

For what it’s worth, I’ve updated the patch and reposted it here:

https://patchwork.freedesktop.org/patch/197091/

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.