Bug 152556

Summary: WebGL based canvases composite incorrectly after changing size
Product: WebKit Reporter: Gregg Tavares <gman>
Component: WebGLAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: adrian, commit-queue, dino, graouts, graouts, kondapallykalyan, pgomez_81, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 9   
Hardware: iPhone / iPad   
OS: iOS 9.0   
URL: http://adam1234.s3-website-us-east-1.amazonaws.com/bug.html
See Also: https://bugs.webkit.org/show_bug.cgi?id=156097
Attachments:
Description Flags
show issue with webgl in ios
none
show canvas 2d works as expected
none
Patch
none
Patch for landing none

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
<rdar://problem/24012678>
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

http://stackoverflow.com/questions/35745878/resize-webgl-context-in-ios
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]
Patch
Comment 10 Dean Jackson 2016-04-14 03:01:51 PDT
Comment on attachment 276378 [details]
Patch

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.