Bug 102679

Summary:	[OpenGL CTS] KHR-GL45.copy_image.functional
Product:	Mesa	Reporter:	Kenneth Graunke <kenneth>
Component:	Drivers/DRI/i965	Assignee:	Kenneth Graunke <kenneth>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	medium	CC:	jasuarez
Version:	git
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	102590

Description Kenneth Graunke 2017-09-12 16:19:38 UTC

KHR-GL45.copy_image.functional fails on i965.  It's copying a GL_RGB10_A2 renderbuffer to a GL_R11F_G11F_B10F 2D array texture.

Looks like a small data difference, or the alpha channel being toast: 0xc7f00004 vs 0xc7f00000.

My guess is that we need to land Jason's BLORP handling for uncommon formats.

Comment 1 Ilia Mirkin 2017-09-15 15:58:08 UTC

Have a look at these 2 patches from Karol:

https://patchwork.freedesktop.org/patch/172106/
https://patchwork.freedesktop.org/patch/172107/

Not sure what the status is, but it's a start. Seems like mostly core issues.

Comment 2 Kenneth Graunke 2017-09-20 08:12:56 UTC

Actually, it looks like we never hit any of the code Karol is modifying - we use BLORP for all glCopyImageSubData operations, with no fallbacks.

I think I misread things earlier, it looks like it's an issue unique to RGB9E5 formats.  If I remove those from the test, it passes.  If I restrict it to just that format, it fails even when copying RGB9E5 to RGB9E5.

Comment 3 Kenneth Graunke 2017-09-20 08:33:04 UTC

Oddly, it seems to only happen for GL_TEXTURE_2D_ARRAY and GL_TEXTURE_3D - other targets are fine.  And the error suggests it's somehow trashing the _source_ image...

Not matching pixels found. Left: [0, 0, 0] lvl:0, off: 0, data: 0xf8fc0000. Right: [0, 0, 0] lvl: 0, off: 0, data: 0xe7e00000

    CopyImageSubData modified contents of source image. Original data: left.

    Failure. Targets src: GL_TEXTURE_2D_ARRAY, dst: GL_TEXTURE_2D_ARRAY. Levels src: 0, dst: 0. Dimmensions src [7, 7], dst [7, 7]. Region [1 x 1 x 12] from [0, 0, 0] to [6, 6, 0]. Format src: 0x8c3d, dst: 0x8c3d

Comment 4 Kenneth Graunke 2017-09-20 08:39:59 UTC

I think this is probably a test bug.  The NVIDIA 384.69 closed source drivers also fail this test, and specifically RGB9E5 subtests.

Comment 5 Kenneth Graunke 2017-10-06 10:21:06 UTC

I was wrong.  While we use BLORP for glCopyImageSubData(), the test also does glGetTexImage() and we fall all the way back to software, and hit the code Karol was fixing.

Karol's memcpy path doesn't work for i965 because src_stride and dst_stride don't match, but if I take the approach Jason mentioned in his review, it totally works.

I'll respin that patch...

Comment 6 Kenneth Graunke 2017-10-07 20:43:27 UTC

Patches on list:
https://patchwork.freedesktop.org/series/31544/

Comment 7 Kenneth Graunke 2017-10-10 22:09:59 UTC

Fixed by:

commit eab078f1321cbd6b292c88fe1770dc1b24261514
Author: Karol Herbst <karolherbst@gmail.com>
Date:   Wed Aug 16 20:32:42 2017 +0200

    main/format: skip format conversion if src and dst format are equal
    
    Fixes 'KHR-GL45.copy_image.functional' on Nouveau and i965.
    
    v2: (by Kenneth Graunke)
        Rewrite patch according to Jason Ekstrand's review feedback.
        This makes it handle differing strides, which i965 needed.
    
    Signed-off-by: Karol Herbst <karolherbst@gmail.com>
    Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

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.