Bug 101704

Summary: [regression][bisected] glReadPixels() from pbuffer failing in Android CTS camera tests
Product: Mesa Reporter: Tomasz Figa <tfiga>
Component: Drivers/DRI/i965Assignee: Intel 3D Bugs Mailing List <intel-3d-bugs>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: chadversary, jason, kenneth, tfiga
Version: git   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Device logcat with test failure
Bisect log

Description Tomasz Figa 2017-07-06 08:06:14 UTC 
Looks like there is a regression affecting glReadPixels() from pbuffer on Android, which manifests in following Android 7.1 CTS tests:

android.hardware.camera2.cts.MultiViewTest#testDualTextureViewAndImageReaderPreview
android.hardware.camera2.cts.MultiViewTest#testTextureViewPreviewWithImageReader
android.hardware.camera2.cts.PerformanceTest#testCameraLaunch
android.hardware.camera2.cts.PerformanceTest#testSingleCapture
android.hardware.camera2.cts.RobustnessTest#testMandatoryOutputCombinations
android.hardware.camera2.cts.SurfaceViewPreviewTest#testSurfaceSet

Log of a failed test attached. I tracked down the exact failure to be a GL_INVALID_OPERATION at

https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/camera2/legacy/SurfaceTextureRenderer.java#796

The error is being generated twice, once by _mesa_get_color_read_format() and then by _mesa_ReadnPixelsARB():

https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/framebuffer.c#n831
https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/readpix.c#n1078


Bisected to:

b7153c3e9f9d2d430b0338313587a00e531e4800 is the first bad commit
commit b7153c3e9f9d2d430b0338313587a00e531e4800
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Fri Jun 9 12:57:09 2017 -0700

    i965: Call intel_prepare_render() from intel_update_state()

Bisect log attached. Revert from top of the branch also fixed the issue.

Might be related to the problem observed in https://bugs.freedesktop.org/show_bug.cgi?id=101558 .

If you have any ideas, I'm open to help in further testing.
Comment 1 Tomasz Figa 2017-07-06 08:07:18 UTC 
Created attachment 132473 [details]
Device logcat with test failure
Comment 2 Tomasz Figa 2017-07-06 08:07:44 UTC 
Created attachment 132474 [details]
Bisect log
Comment 3 Kenneth Graunke 2017-07-20 23:13:12 UTC 
Should be fixed:

commit 8696c3e997d23f65ade53df8f7efcfb21a99fb3d
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Wed Jul 19 21:29:51 2017 -0700

    Revert "i965: Call intel_prepare_render() from intel_update_state()"
    
    This reverts commit b7153c3e9f9d2d430b0338313587a00e531e4800.
    
    The point of that commit was to ensure intel_prepare_render() occurred
    before color resolves on the current framebuffer.  In 0673bbfd9ba16be8
    (i965: Move surface resolves back to draw/dispatch time), Jason moved
    brw_predraw_resolve_framebuffer back to draw time, which is already
    after a intel_prepare_render() call.  So, this is no longer necessary.
    
    Furthermore, it caused problems.  "mpv" would only display a small
    corner of movies, and Android started failing camera CTS tests.
    
    This is because intel_prepare_render() ended up handling DRI2 events
    which caused the drawable to be resized at an inopportune time, flagging
    ctx->NewState |= _NEW_BUFFERS, but at a point where we've already copied
    ctx->NewState, and failed to notice the newly set flag.
    
    The lack of _NEW_BUFFERS caused us to skip 3DSTATE_DRAWING_RECTANGLE,
    so the drawing ended up being clipped to an outdated framebuffer size.
    
    Just drop the hack and go back to handling this at the proper time.
    
    Thanks to Matti Hämäläinen (ccr), Tomasz Figa (tfiga), and Tapani Palli
    for reporting these issues.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101558
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101704
    Tested-by: Tapani Pälli <tapani.palli@intel.com>

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.