Bug 215141

Summary: Flickering on sedona.dev
Product: WebKit Reporter: Dean Jackson <dino>
Component: WebGLAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, darin, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, jdarpinian, kbr, kondapallykalyan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Web Inspector
none
Patch darin: review+

Description Dean Jackson 2020-08-04 14:50:35 PDT
* TITLE:
Safari Technology Preview canvas flickering (not on Safari)

* USER REPORTED AREA:
Safari

* MACHINE WAS RUNNING BUILD:
unknown

* MACHINE MODEL:
Web

* DESCRIPTION:
Visit the https://sedona.dev/ using the latest Safari Technology Release [Release 110 (Safari 14.0, WebKit 15610.1.21.0.2)]. Hovering and moving over the canvas will produce flickering not seen in the most recent version of Safari [Version 13.1.2 (15609.3.5.1.3)], Chrome or Firefox.

A little bit of code (changing the alpha and premultipliedAlpha affect the colour of the flickering, but do not reduce it.):
    let renderer = new THREE.WebGLRenderer({
      antialias: false,
      //alpha: true,
      //premultipliedAlpha: false
    });

    renderer.setPixelRatio(window.devicePixelRatio);
    renderer.setSize(vm.width, vm.height);
    var ctx = renderer.getContext();

    vm.pickingScene = new THREE.Scene();
    vm.pickingScene.autoUpdate = false;
    vm.pickingTexture = new THREE.WebGLRenderTarget(1, 1);

    function updateIntersects() {
      if(ctx.checkFramebufferStatus(ctx.FRAMEBUFFER) !== ctx.FRAMEBUFFER_COMPLETE)
        return;

      if (!vm.mousePos.x || !vm.mousePos.y)
        return;

      const pixelBuffer = new Uint8Array(4);

      const pixelRatio = vm.renderer.getPixelRatio();

      vm.camera.setViewOffset(
        ctx.drawingBufferWidth,
        ctx.drawingBufferHeight,
        (vm.mousePos.x * pixelRatio) | 0,
        (vm.mousePos.y * pixelRatio) | 0,
        1,
        1
      );

      vm.renderer.setRenderTarget(vm.pickingTexture);
      vm.renderer.render(vm.pickingScene, vm.camera);
      renderer.setRenderTarget(null);

      vm.camera.clearViewOffset();

      vm.renderer.readRenderTargetPixels(
        vm.pickingTexture,
        0,
        0,
        1,
        1,
        pixelBuffer
      );

      const id =
        (pixelBuffer[0] << 16) | (pixelBuffer[1] << 8) | (pixelBuffer[2] << 0);

      if (pixelBuffer[3] > 0 && id >= 0 && id < vm.indexedVertices.length) {
        const v = vm.indexedVertices[id];

        if (vm.activeVertex !== v) {
          vm.activeVertex = v;
        }

* OTHER INFORMATION:

Safari issue area
Webpage Rendering

Specific URL
https://sedona.dev/

What extensions or content blockers do you have enabled?
N/A
Comment 1 Dean Jackson 2020-08-04 14:51:22 PDT
This reproduces for me in Safari Technology Preview. On a trunk build I don't get any visual output at all. Lots of "Starting Worker" in the console.
Comment 2 Dean Jackson 2020-08-04 14:51:36 PDT
<rdar://66034097>
Comment 3 Kenneth Russell 2020-08-04 15:39:59 PDT
This page comes up blank in Safari 13.1.1 (15609.2.9.1.2) on my 2017 15" MacBook Pro with dual Intel HD 630 and AMD Radeon Pro 560 GPUs. It also comes up blank in Safari Technology Preview Release 109 (Safari 14.0, WebKit 15610.1.17.2) .
Comment 4 Alexey Proskuryakov 2020-08-04 17:20:20 PDT
I can reproduce on a 16" MacBook Pro with macOS 10.15.6 and Safari 14 developer beta.
Comment 5 Dean Jackson 2020-08-11 18:33:10 PDT
Still getting blank. However if I readPixels, there is WebGL content being drawn. This appears to be a compositing problem.
Comment 6 Dean Jackson 2020-08-11 18:43:51 PDT
Created attachment 406441 [details]
Web Inspector

As you can see, the Web Inspector believes the WebGL is being drawn. I can confirm that prepareForDisplay and display are being called on WebGLLayer. But for some reason nothing is appearing.
Comment 7 Dean Jackson 2020-08-11 18:47:11 PDT
It takes a while in a debug build, but eventually the simulation settles and it stops drawing frames.

But still nothing is appearing.
Comment 8 Dean Jackson 2020-08-11 18:50:53 PDT
Even if I grab the canvas with the inspector and do this, nothing appears:

let _ctx = $0.getContext("webgl")
// confirm it is a WebGL2RenderingContext

_ctx.clearColor(1, 0, 0, 1);
_ctx.clear(_ctx.COLOR_BUFFER_BIT);
Comment 9 Dean Jackson 2020-08-11 18:53:43 PDT
Oh wait. A LOT of page content isn't showing - not just WebGL.
Comment 10 Dean Jackson 2020-08-11 19:03:12 PDT
Seems to be an issue where the .row object has a height of zero, despite children.

If I give it an explicit height, page content appears. The canvas is still hidden though.

If I give the canvas a background-color, all of a sudden the WebGL appears.

I'll try autospading this for a moment.
Comment 11 Dean Jackson 2020-08-11 19:27:49 PDT
Display issue is in this range: http://trac.webkit.org/log/trunk/?mode=follow_copy&rev=264775&stop_rev=264772
Comment 12 Dean Jackson 2020-08-11 19:30:46 PDT
Seems to be https://bugs.webkit.org/show_bug.cgi?id=214655
which is a revert of http://trac.webkit.org/changeset/262124/webkit

Although Sergio says it was originally
https://bugs.webkit.org/show_bug.cgi?id=210089
Comment 13 Dean Jackson 2020-08-11 19:31:56 PDT
I wouldn't be surprised if the WebGL still flickers if the flex bug is fixed.
Comment 14 Dean Jackson 2020-08-11 19:40:51 PDT
It does still flicker with the flexbox fix un-reverted.

And it looks like WebGL is really drawing a completely black canvas.
Comment 15 Dean Jackson 2020-08-12 02:27:31 PDT
For the black frames there is a call to

context.drawArrays(POINTS, 0, 4942);

that does not render anything. But that same call does render correctly on the non-black frames.

I think something is confused when the app switches framebuffers, which it does every frame. In fact, once the graph becomes stable, it doesn't appear to draw any geometry.
Comment 16 Dean Jackson 2020-08-18 17:21:40 PDT
I'm a bit confused as to what is going on with the page. On good frames the page simply does:

bufferSubData
clear
drawArrays (the lines)
drawArrays (the nodes)

On the frames that produce black, there is also a pass that:

createFramebuffer
createTexture
texImage2D
framebufferTexture2D
createRenderbuffer
viewport
scissor
createProgram
adds shaders and links
drawArrays
readPixels

The page is compiled, so it is hard to work out what is going on, but it seems to be coming from a call to three.js readRenderTargetPixels which I think comes from the three.js mouse/target picking code. This would explain why it happens as you move the mouse.
Comment 17 Dean Jackson 2020-08-18 17:33:22 PDT
Yes, there is a flag mouseNeedsUpdate in the rAF loop. I'm not sure if I'm in three.js code or the page code.

What's quite nice is that if you let the graph get stable, I think you can trigger a bad frame just by moving the mouse, breaking in the debugger, and moving the mouse out of the window before continuing.
Comment 18 Dean Jackson 2020-08-18 17:53:36 PDT
Doesn't reproduce in Chrome (with the ANGLE backend)
Comment 19 Dean Jackson 2020-08-18 19:31:18 PDT
I have a fix. We are triggering a composite in a state where we've already done it, and thus swapping a blank texture in for the current one.
Comment 20 Dean Jackson 2020-08-19 13:36:27 PDT
I'm just trying to work out what causes us to get into this state.
Comment 21 Dean Jackson 2020-08-19 19:07:01 PDT
Created attachment 406900 [details]
Patch
Comment 22 Kenneth Russell 2020-08-19 23:36:59 PDT
Comment on attachment 406900 [details]
Patch

Yikes, good catch. We really should add WPT reftests for WebGL to catch scenarios like this.
Comment 23 Kenneth Russell 2020-08-19 23:39:25 PDT
Commented on https://github.com/KhronosGroup/WebGL/issues/2527 pointing back to this bug as an example of one that could have been caught with a reftest.
Comment 24 Dean Jackson 2020-08-20 16:41:23 PDT
Making a ref test for this is a good idea. In order to avoid any implementation details (e.g. how many swap buffers you have), I guess it could render a small number of frames each with a different colour, enough to ensure the swap has been filled. Then render to an FBO without touching the main canvas.
Comment 25 Kenneth Russell 2020-08-20 16:42:53 PDT
Any thoughts on the mac-debug-wk1 failures? Many of the tests are triggering the new assertion in GraphicsContextGLOpenGL::prepareTexture.
Comment 26 Dean Jackson 2020-08-20 16:43:53 PDT
In fact, given the wk1 crashes, I might have to do that right away.

It looks like the difference in compositing timing for WK1 means that my assert is not valid.
Comment 27 Dean Jackson 2020-08-20 16:44:51 PDT
Yeah, the crashes are an ASSERT I added that was designed to be caught by the test (without the patch). I'll investigate why it was invalid, but also see if I can make this a reftest.
Comment 28 Dean Jackson 2020-08-20 19:40:03 PDT
Huh. Of course the tests don't fail in Minibrowser WK1, so it is again something mysterious with the way DumpRenderTree bypasses the regular page update cycle. I already had to work around this once before, when originally implementing the "prepare at the end of drawing but before compositing" code.
Comment 29 Dean Jackson 2020-08-24 16:32:58 PDT
For some reason WebGLLayer `display` is called twice in a row, without any drawing happening. I'm not sure what would cause it to be marked with setNeedsDisplay.
Comment 30 Kenneth Russell 2020-08-24 16:41:43 PDT
Is setNeedsDisplay being called on the WebGLLayer without your newly-introduced [WebGLLayer prepareForDisplay] being called?
Comment 31 Dean Jackson 2020-08-25 15:57:53 PDT
WebCore::ResourceLoader::didFinishLoading seems to be called twice, which triggers a ContentsPlatformLayerChanged (forcing another CA 'display')
Comment 32 Dean Jackson 2020-08-25 16:00:47 PDT
Multiple calls to WebCore::FrameView::forceLayout.

I guess this means that setContentsToPlatformLayer is actually moving the WebGLLayer to somewhere new.
Comment 33 Dean Jackson 2020-08-25 16:33:30 PDT
I think my assumption is slightly wrong. At least with DumpRenderTree I'm seeing display being called before prepareDisplay for some WebGL content.

That's because didFinishLoading calls forceLayout which calls setContentsToPlatformLayer which is noticed by updateContentsPlatformLayer and it calls setNeedsDisplay directly, thus bypassing the code we have to prepare things in doAfterUpdateRendering.

I'll ask Simon what he suggests.
Comment 34 Dean Jackson 2020-08-26 13:42:50 PDT
Committed r266189: <https://trac.webkit.org/changeset/266189>