REGRESSION (r262366): [ Mac wk1 ] webgl/webgl-backing-store-size-update.html is failing
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jun 2020 19:52:20 +0000 (19:52 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jun 2020 19:52:20 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212647
<rdar://problem/63882960>

Reviewed by Eric Carlson.

Source/WebCore:

In some unusual cases, specifically WebKit 1, a WebGLLayer could prepareForDisplay
but never get told to actually display, producing incorrect results. Fix this by
simply calling setNeedsDisplay.

While here, address some comments from Simon Fraser that were made
after r262366.

* dom/Document.cpp:
(WebCore::Document::prepareCanvasesForDisplayIfNeeded):
* platform/graphics/cocoa/WebGLLayer.h: Move the instance variables into the implementation file
since they are not exposed as an interface.
* platform/graphics/cocoa/WebGLLayer.mm:
(-[WebGLLayer initWithGraphicsContextGL:]):
(-[WebGLLayer prepareForDisplay]):
(-[WebGLLayer display]):

Tools:

Drive-by comment from Simon. Remove some useless braces.

* TestWebKitAPI/Tests/WebKitLegacy/ios/WebGLPrepareDisplayOnWebThread.mm:

LayoutTests:

Make this test wait a frame before calling notifyDone. This is required at the
moment because calling notifyDone in WebKit1 sidesteps the normal painting logic and
forces compositing to happen, and thus will produce incorrect results for WebGL if
we're already in the middle of the rendering loop (e.g. inside a requestAnimationFrame
that has touched pixels).

* webgl/webgl-backing-store-size-update.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@262643 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webgl/webgl-backing-store-size-update.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/platform/graphics/cocoa/WebGLLayer.h
Source/WebCore/platform/graphics/cocoa/WebGLLayer.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/WebGLPrepareDisplayOnWebThread.mm

index 78c2a4f..1cf9fa5 100644 (file)
@@ -1,3 +1,19 @@
+2020-06-05  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r262366): [ Mac wk1 ] webgl/webgl-backing-store-size-update.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=212647
+        <rdar://problem/63882960>
+
+        Reviewed by Eric Carlson.
+
+        Make this test wait a frame before calling notifyDone. This is required at the
+        moment because calling notifyDone in WebKit1 sidesteps the normal painting logic and
+        forces compositing to happen, and thus will produce incorrect results for WebGL if
+        we're already in the middle of the rendering loop (e.g. inside a requestAnimationFrame
+        that has touched pixels).
+
+        * webgl/webgl-backing-store-size-update.html:
+
 2020-06-05  Peng Liu  <peng.liu6@apple.com>
 
         REGRESSION (r262456?): [macOS] media/picture-in-picture/picture-in-picture-api tests are flaky timeouts
index f178a9b..b70693d 100644 (file)
     canvas.style.height = "50px";
     window.requestAnimationFrame(function() {
         draw();
-        if (window.testRunner)
-            testRunner.notifyDone();
+        if (window.testRunner) {
+            // Need to give the test system at least a frame to paint
+            // after the draw() command, because notifyDone forces a
+            // dump() and we're inside a rAF (which means it won't
+            // do any of the post-layout/drawing work in Page::doAfterUpdateRendering)
+            window.requestAnimationFrame(function() {
+                testRunner.notifyDone();
+            });
+        }
     });
 
 </script>
index 9f7fd45..bd0bae8 100644 (file)
@@ -1,3 +1,27 @@
+2020-06-05  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r262366): [ Mac wk1 ] webgl/webgl-backing-store-size-update.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=212647
+        <rdar://problem/63882960>
+
+        Reviewed by Eric Carlson.
+
+        In some unusual cases, specifically WebKit 1, a WebGLLayer could prepareForDisplay
+        but never get told to actually display, producing incorrect results. Fix this by
+        simply calling setNeedsDisplay.
+
+        While here, address some comments from Simon Fraser that were made
+        after r262366.
+
+        * dom/Document.cpp:
+        (WebCore::Document::prepareCanvasesForDisplayIfNeeded):
+        * platform/graphics/cocoa/WebGLLayer.h: Move the instance variables into the implementation file
+        since they are not exposed as an interface.
+        * platform/graphics/cocoa/WebGLLayer.mm:
+        (-[WebGLLayer initWithGraphicsContextGL:]):
+        (-[WebGLLayer prepareForDisplay]):
+        (-[WebGLLayer display]):
+
 2020-06-05  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, silence deprecation warning to fix build with latest SDK.
index 99539c6..ec8b86b 100644 (file)
@@ -8594,8 +8594,8 @@ void Document::prepareCanvasesForDisplayIfNeeded()
         if (!canvas->isInTreeScope())
             continue;
 
-        auto refCountedCanvas = makeRefPtr(canvas);
-        refCountedCanvas->prepareForDisplay();
+        auto protectedCanvas = makeRefPtr(canvas);
+        protectedCanvas->prepareForDisplay();
     }
     m_canvasesNeedingDisplayPreparation.clear();
 }
index 5501566..28729b1 100644 (file)
@@ -44,26 +44,6 @@ ALLOW_DEPRECATED_DECLARATIONS_BEGIN
 #else
 #error Unsupported platform
 #endif
-{
-    NakedPtr<WebCore::GraphicsContextGLOpenGL> _context;
-    float _devicePixelRatio;
-#if USE(OPENGL) || USE(ANGLE)
-    std::unique_ptr<WebCore::IOSurface> _contentsBuffer;
-    std::unique_ptr<WebCore::IOSurface> _drawingBuffer;
-    std::unique_ptr<WebCore::IOSurface> _spareBuffer;
-    WebCore::IntSize _bufferSize;
-    BOOL _usingAlpha;
-#endif
-#if USE(ANGLE)
-    void* _eglDisplay;
-    void* _eglConfig;
-    void* _contentsPbuffer;
-    void* _drawingPbuffer;
-    void* _sparePbuffer;
-    void* _latchedPbuffer;
-#endif
-    BOOL _prepared;
-}
 
 @property (nonatomic) NakedPtr<WebCore::GraphicsContextGLOpenGL> context;
 
index 2868517..78a6f03 100644 (file)
@@ -78,11 +78,27 @@ namespace {
     };
 }
 
-@implementation WebGLLayer
-
-@synthesize context=_context;
+@implementation WebGLLayer {
+    float _devicePixelRatio;
+#if USE(OPENGL) || USE(ANGLE)
+    std::unique_ptr<WebCore::IOSurface> _contentsBuffer;
+    std::unique_ptr<WebCore::IOSurface> _drawingBuffer;
+    std::unique_ptr<WebCore::IOSurface> _spareBuffer;
+    WebCore::IntSize _bufferSize;
+    BOOL _usingAlpha;
+#endif
+#if USE(ANGLE)
+    void* _eglDisplay;
+    void* _eglConfig;
+    void* _contentsPbuffer;
+    void* _drawingPbuffer;
+    void* _sparePbuffer;
+    void* _latchedPbuffer;
+#endif
+    BOOL _preparedForDisplay;
+}
 
--(id)initWithGraphicsContextGL:(NakedPtr<WebCore::GraphicsContextGLOpenGL>)context
+- (id)initWithGraphicsContextGL:(NakedPtr<WebCore::GraphicsContextGLOpenGL>)context
 {
     _context = context;
     self = [super init];
@@ -204,8 +220,8 @@ static void freeData(void *, const void *data, size_t /* size */)
         [self bindFramebufferToNextAvailableSurface];
     }
 #endif
-
-    _prepared = YES;
+    [self setNeedsDisplay];
+    _preparedForDisplay = YES;
 }
 
 - (void)display
@@ -218,7 +234,7 @@ static void freeData(void *, const void *data, size_t /* size */)
     // This avoids running any OpenGL code in this method.
 
 #if USE(OPENGL) || USE(ANGLE)
-    if (_contentsBuffer && _prepared) {
+    if (_contentsBuffer && _preparedForDisplay) {
         self.contents = _contentsBuffer->asLayerContents();
         [self reloadValueForKeyPath:@"contents"];
     }
@@ -231,7 +247,7 @@ static void freeData(void *, const void *data, size_t /* size */)
     if (layer && layer->owner())
         layer->owner()->platformCALayerLayerDidDisplay(layer.get());
 
-    _prepared = NO;
+    _preparedForDisplay = NO;
 }
 
 #if USE(ANGLE)
index 730e33b..028b377 100644 (file)
@@ -1,3 +1,15 @@
+2020-06-05  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r262366): [ Mac wk1 ] webgl/webgl-backing-store-size-update.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=212647
+        <rdar://problem/63882960>
+
+        Reviewed by Eric Carlson.
+
+        Drive-by comment from Simon. Remove some useless braces.
+
+        * TestWebKitAPI/Tests/WebKitLegacy/ios/WebGLPrepareDisplayOnWebThread.mm:
+
 2020-06-05  Saam Barati  <sbarati@apple.com>
 
         Try to reduce jetsams further on iOS devices running jsc stress tests
index 1c4dae6..b8b0c7b 100644 (file)
 #import <stdlib.h>
 #import <wtf/RetainPtr.h>
 
-@interface WebGLPrepareDisplayOnWebThreadDelegate : NSObject <UIWebViewDelegate> {
-}
-@end
-
 static bool didFinishLoad = false;
 static bool didFinishPainting = false;
 static bool isReady = false;
 
+@interface WebGLPrepareDisplayOnWebThreadDelegate : NSObject <UIWebViewDelegate>
+@end
+
 @implementation WebGLPrepareDisplayOnWebThreadDelegate
 
 IGNORE_WARNINGS_BEGIN("deprecated-implementations")