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.
<rdar://problem/34597567>
Created attachment 323996 [details] Patch
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.
Committed r223459: <https://trac.webkit.org/changeset/223459>
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?
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
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
Reverted r223459 for reason: This change introduced LayoutTest failures. Committed r223563: <https://trac.webkit.org/changeset/223563>
Committed r223567: <https://trac.webkit.org/changeset/223567>
(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?
(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.
(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
*** Bug 162604 has been marked as a duplicate of this bug. ***