Bug 183151 - Bad flicker on three.js example
Summary: Bad flicker on three.js example
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
Keywords: InRadar
Depends on: 159608 211602 211758
Blocks: 168623 209510 211748
  Show dependency treegraph
Reported: 2018-02-26 15:04 PST by Dean Jackson
Modified: 2020-05-13 13:32 PDT (History)
9 users (show)

See Also:

Patch (26.13 KB, patch)
2020-05-08 14:28 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff
Zipped up webgl_trails for easier debugging (235.29 KB, application/zip)
2020-05-08 22:22 PDT, Kenneth Russell
no flags Details
Patch (12.78 KB, patch)
2020-05-13 11:47 PDT, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Radar WebKit Bug Importer 2018-02-26 15:05:18 PST
Comment 2 Dean Jackson 2018-02-26 15:06:11 PST
It's likely that preserveDrawingBuffer was broken as I changed compositing.
Comment 3 Kenneth Russell 2020-05-08 13:29:21 PDT
On top-of-tree WebKit, it looks like the only scenario which is definitively broken is preserveDrawingBuffer:true with antialias:false. Can confirm that a fix of this code path in the ANGLE backend eliminates this flicker on desktop macOS.

This is required to fix the conformance2webgl/2.0.0/conformance2/textures/webgl_canvas tests; blocking the umbrella bug for that.
Comment 4 Kenneth Russell 2020-05-08 14:28:29 PDT
Created attachment 398887 [details]
Comment 5 Dean Jackson 2020-05-08 16:25:09 PDT
When I compile and run this patch on iOS hardware I don't get any rendering on https://rawgit.com/mrdoob/three.js/dev/examples/webgl_trails.html (just black).

I confirmed it still flickered without the patch.
Comment 6 Kenneth Russell 2020-05-08 16:31:27 PDT
Comment on attachment 398887 [details]

Rejecting review because this patch doesn't work on iOS hardware. Debugging.
Comment 7 Kenneth Russell 2020-05-08 18:35:53 PDT
The black screen reproduces on the iOS Simulator, too.

ANGLE takes the CopyTexSubImage2D path in BlitGL::copyTexSubImage for this case. Not immediately sure why that's not working.
Comment 8 Kenneth Russell 2020-05-08 22:22:35 PDT
Created attachment 398917 [details]
Zipped up webgl_trails for easier debugging
Comment 9 Kenneth Russell 2020-05-08 22:28:11 PDT
The problem on iOS is related to alpha:false. Setting it to alpha:true makes the content appear.
Comment 10 Kenneth Russell 2020-05-08 22:48:37 PDT
Debugging this earlier, for the alpha:false case, TextureGL::copySubTextureHelper was going down the BlitGL::copyTexSubImage path - didn't see exactly the source and destination internal formats.

For the alpha:true case, the sized internal format of the destination texture is GL_BGRA8_EXT, which in BuildInternalFormatInfoMap() in src/libANGLE/renderer/gl/formatutilsgl.cpp is declared not renderable on GLES - this is clearly wrong on iOS. It's falling back to CPU readback which seems to be why this case is working at all on iOS.
Comment 11 Kenneth Russell 2020-05-13 11:47:10 PDT
Created attachment 399286 [details]
Comment 12 Kenneth Russell 2020-05-13 11:47:53 PDT
From the ChangeLog:

        With preserveDrawingBuffer:true and antialias:false, allocate an
        intermediate texture and FBO, and blit from it to the destination
        texture in prepareTexture(). Use wipeAlphaChannelFromPixels on iOS
        as well as macOS.

        In addition to fixing the test case from the bug, this also fixes
        the webgl/2.0.0/conformance2/textures/webgl_canvas/ layout tests,
        which will be re-enabled in a subsequent patch. It also passes the
        more stringent webgl_canvas conformance tests in
        https://github.com/KhronosGroup/WebGL/pull/3071 .
Comment 13 Dean Jackson 2020-05-13 12:43:55 PDT
Comment on attachment 399286 [details]

I tested this on a device and confirmed the flicker is gone.
Comment 14 EWS 2020-05-13 12:57:10 PDT
Committed r261639: <https://trac.webkit.org/changeset/261639>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399286 [details].