Bug 152556 - WebGL based canvases composite incorrectly after changing size
Summary: WebGL based canvases composite incorrectly after changing size
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Safari 9
Hardware: iPhone / iPad iOS 9.0
: P2 Normal
Assignee: Antoine Quint
URL: http://adam1234.s3-website-us-east-1....
Keywords: InRadar
Depends on:
Reported: 2015-12-24 23:13 PST by Gregg Tavares
Modified: 2016-06-02 20:37 PDT (History)
9 users (show)

See Also:

show issue with webgl in ios (670 bytes, text/html)
2015-12-24 23:13 PST, Gregg Tavares
no flags Details
show canvas 2d works as expected (596 bytes, text/html)
2015-12-24 23:14 PST, Gregg Tavares
no flags Details
Patch (6.66 KB, patch)
2016-04-14 01:26 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (6.22 KB, patch)
2016-04-14 03:10 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gregg Tavares 2015-12-24 23:13:41 PST
Created attachment 267913 [details]
show issue with webgl in ios

Set a canvas's CSS width and height to 100% inside some fixed size container.
From JavaScript change the canvas's width and height (its drawingbuffer size). 
The canvas should still be displayed stretched to its container's width and height. It's not

This works with 2d canvas. It only fails on webgl canvas.

Also it's iOS only. Works find on desktop safari

Run the attached sample, ios-webgl-issue.html: You should see a red square filled with green, then 2 seconds later filled with orange. On iOS the orange isn't being stretched.

The ios-canvas-issue shows 2d canvas works correctly.

Other things to note: The inspector claims the the canvas is the correct size (stretched to fit). Also JavaScript claims the canvas is the correct size (stretched to fit). canvas.clientWidth returns 400 and canvas.getBoundingClientRect returns a stretched size
Comment 1 Gregg Tavares 2015-12-24 23:14:14 PST
Created attachment 267914 [details]
show canvas 2d works as expected
Comment 2 Gregg Tavares 2015-12-24 23:19:37 PST
Also note, setting the containers CSS dimensions fixes the issue although it appears compositing has to happen at least once. In other words

    container.style.width = "401px"; container.style.width = "400px";

doesn't work but
    container.style.width = "401px";  // change container size temporarily
    setTimeout(function() {
      container.style.width = "400px":  // change it back
    }, 200);

Sounds like some internal flag just needs to be set to fix this
Comment 3 Radar WebKit Bug Importer 2015-12-27 10:59:24 PST
Comment 4 adrian 2016-03-23 07:30:31 PDT
I have also been having this problem and its really serious for us as the webgl renderer in retina mode draws outside of the canvas element!  At least one other person has also had this difficulty

Comment 5 Simon Fraser (smfr) 2016-03-23 08:43:13 PDT
Do you have a simple testcase?
Comment 6 Simon Fraser (smfr) 2016-03-23 09:12:35 PDT
Never mind, I found http://adam1234.s3-website-us-east-1.amazonaws.com/bug.html
Comment 7 Simon Fraser (smfr) 2016-04-04 11:44:35 PDT
Only happens on iOS, and when the canvas backing store has a different size to the canvas.
Comment 8 Simon Fraser (smfr) 2016-04-04 12:10:17 PDT
GraphicsContext3D::setRenderbufferStorageFromDrawable() is setting the WebGL height/width to the backing store size, but nothing causes that WebGL layer to respect the contentsRect of its GraphicsLayer.
Comment 9 Antoine Quint 2016-04-14 01:26:35 PDT
Created attachment 276378 [details]
Comment 10 Dean Jackson 2016-04-14 03:01:51 PDT
Comment on attachment 276378 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=276378&action=review

> Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:346
> +    // do not want to clobber the bounds set during layout, so we remember the current value,

I think the comments after "layout," aren't needed.

> Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:348
> +    CGRect previousBounds = m_webGLLayer.get().bounds;

I think we tend to write these as [a.get() b] 
If we didn't have the refptr in the way it would be fine as a.b

> LayoutTests/webgl/webgl-backing-store-size-update-expected.html:6
> +<div style="width: 50px; height: 50px; background-color: black;"></div>

You only need this line.

> LayoutTests/webgl/webgl-backing-store-size-update.html:6
> +<canvas style="width:100px; height:100px;">

You don't need anything before this line.

> LayoutTests/webgl/webgl-backing-store-size-update.html:16
> +        var devicePixelRatio = 2;

Technically i think we just want to make sure that the width and height don't match the layout size. It isn't really DPR.

> LayoutTests/webgl/webgl-backing-store-size-update.html:22
> +        gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);

You only need to clear the COLOR part in this example.
Comment 11 Antoine Quint 2016-04-14 03:10:28 PDT
Created attachment 276385 [details]
Patch for landing
Comment 12 WebKit Commit Bot 2016-04-14 04:09:29 PDT
Comment on attachment 276385 [details]
Patch for landing

Clearing flags on attachment: 276385

Committed r199536: <http://trac.webkit.org/changeset/199536>
Comment 13 WebKit Commit Bot 2016-04-14 04:09:34 PDT
All reviewed patches have been landed.  Closing bug.