Bug 178223 – WebGL clamps drawingBufferWidth to 4096 pixels on a 5120 monitor/canvas

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Alec Miller]

Created attachment 323543 [details]
Example showing clamping of WebGL renderBuffer

Safari currently clamps the WebGL context to 4096 pixels horizontally and vertically.  This is what Chrome/Firefox used to do, but they made the clamp on total pixels (4k x 4k) instead of per side.  It looks like the WebKit implementation is using the old behavior.  This is problematic on both 5K displays including the 5K iMac.

Enclosed is a simple WebGL sample.  Expand the window on a 5K display, and at a canvas size of 4097 pixels, you'll see a jump in the color and then aliasing bands.  The drawingBufferWidth is clamped to 4096 pixels, and the checkerboard pattern is non-uniformly scaled up to 5120 pixels if you expand the browser fully or go fullscreen.

This is problematic for design tools like ours, where we rely on pixel precision.  Now the experience on a 5K display will be letterboxed bars around the interactive canvas when the UI is hidden.  You can see this in figma.com rendering on 5K monitor with "Show UI" disabled.  

We would prefer to not have to wait for a long update cycle (another OSX release) on WebKit to see this corrected.

[Dean Jackson]

<rdar://problem/34597567>

[Dean Jackson]

Created attachment 323996 [details]
Patch

[Antoine Quint]

Comment on attachment 323996 [details]
Patch

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

> LayoutTests/fast/canvas/webgl/large-drawing-buffer-resize.html:56
> +}, false);

You can drop this parameter.

[Dean Jackson]

Committed r223459: <https://trac.webkit.org/changeset/223459>

[Jon Lee]

Comment on attachment 323996 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:-851
> -    maxSize = std::min(maxSize, sizeUpperLimit);

Why was this limit here in the first place? Should we restrict the size based on the device?

[Ryan Haddad]

LayoutTest webgl/1.0.2/conformance/canvas/drawingbuffer-static-canvas-test.html is failing after this change:

https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK1%20(Tests)/r223462%20(5490)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=webgl%2F1.0.2%2Fconformance%2Fcanvas%2Fdrawingbuffer-static-canvas-test.html

[Ryan Haddad]

The test for this change is timing out on iOS 11 Release bots, failing on iOS 11 Debug:

Release:
https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r223553%20(595)/results.html

Debug:
https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/425
--- /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test-results/fast/canvas/webgl/large-drawing-buffer-resize-expected.txt
+++ /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test-results/fast/canvas/webgl/large-drawing-buffer-resize-actual.txt
@@ -1,5 +1,5 @@
-Original drawing buffer: 5000 x 5000
+Original drawing buffer: 4096 x 4096
 
-Resized drawing buffer: 5001 x 5001
+Resized drawing buffer: 4096 x 4096

[Ryan Haddad]

Reverted r223459 for reason:

This change introduced LayoutTest failures.

Committed r223563: <https://trac.webkit.org/changeset/223563>

[Dean Jackson]

Committed r223567: <https://trac.webkit.org/changeset/223567>

[Ryan Haddad]

(In reply to Dean Jackson from comment #9)
> Committed r223567: <https://trac.webkit.org/changeset/223567>
Thanks for updating the TestExpectations for iOS. Was anything done to address the webgl/1.0.2/conformance/canvas/drawingbuffer-static-canvas-test.html failure on macOS?

[Dean Jackson]

(In reply to Ryan Haddad from comment #10)
> (In reply to Dean Jackson from comment #9)
> > Committed r223567: <https://trac.webkit.org/changeset/223567>
> Thanks for updating the TestExpectations for iOS. Was anything done to
> address the
> webgl/1.0.2/conformance/canvas/drawingbuffer-static-canvas-test.html failure
> on macOS?

No, I didn't see that one. I'll fix it now.

[Dean Jackson]

(In reply to Jon Lee from comment #5)
> Comment on attachment 323996 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=323996&action=review
> 
> > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:-851
> > -    maxSize = std::min(maxSize, sizeUpperLimit);
> 
> Why was this limit here in the first place? Should we restrict the size
> based on the device?

I think this was mostly there because older hardware had problems with large buffers, but we still identify that hardware in another place.

Note that this was only being applied on resize, so it was still possible to create such large buffers. i.e. we were not protecting it properly anyway

[Michael Herzog]

*** Bug 162604 has been marked as a duplicate of this bug. ***