Diff
Modified: trunk/LayoutTests/ChangeLog (266442 => 266443)
--- trunk/LayoutTests/ChangeLog 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/LayoutTests/ChangeLog 2020-09-02 01:46:53 UTC (rev 266443)
@@ -1,3 +1,16 @@
+2020-09-01 Dean Jackson <[email protected]>
+
+ REGRESSION(r262366): MotionMark1.1 | macOS | Some devices | 1-3% overall regression
+ https://bugs.webkit.org/show_bug.cgi?id=215989
+ <rdar://problem/66845937>
+
+ Reviewed by Darin Adler.
+
+ * fast/canvas/webgl/move-canvas-in-document-expected.html: Added.
+ * fast/canvas/webgl/move-canvas-in-document-while-clean-expected.html: Added.
+ * fast/canvas/webgl/move-canvas-in-document-while-clean.html: Added.
+ * fast/canvas/webgl/move-canvas-in-document.html: Added.
+
2020-09-01 Hector Lopez <[email protected]>
[ macOS wk2 ] fast/scrolling/latching/latched-scroll-remove-iframe.html is a flaky failure
Added: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-expected.html (0 => 266443)
--- trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-expected.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-expected.html 2020-09-02 01:46:53 UTC (rev 266443)
@@ -0,0 +1,29 @@
+<style>
+canvas {
+ width: 10px;
+ height: 10px;
+}
+</style>
+<script>
+function runTest() {
+ // Step 1. Grab the WebGL canvas and render a solid color.
+
+ const canvas = document.querySelector("canvas");
+ canvas.width = 10;
+ canvas.height = 10;
+
+ const gl = canvas.getContext("webgl");
+
+ // Clear to green
+
+ gl.clearColor(0, 1, 0, 1);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+}
+
+window.addEventListener("load", runTest, false);
+</script>
+<div id="container1">
+</div>
+<div id="container2">
+<canvas></canvas>
+</div>
Property changes on: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-expected.html
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Added: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean-expected.html (0 => 266443)
--- trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean-expected.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean-expected.html 2020-09-02 01:46:53 UTC (rev 266443)
@@ -0,0 +1,29 @@
+<style>
+canvas {
+ width: 10px;
+ height: 10px;
+}
+</style>
+<script>
+function runTest() {
+ // Step 1. Grab the WebGL canvas and render a solid color.
+
+ const canvas = document.querySelector("canvas");
+ canvas.width = 10;
+ canvas.height = 10;
+
+ const gl = canvas.getContext("webgl");
+
+ // Clear to green
+
+ gl.clearColor(0, 1, 0, 1);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+}
+
+window.addEventListener("load", runTest, false);
+</script>
+<div id="container1">
+</div>
+<div id="container2">
+<canvas></canvas>
+</div>
Property changes on: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean-expected.html
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Added: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean.html (0 => 266443)
--- trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean.html 2020-09-02 01:46:53 UTC (rev 266443)
@@ -0,0 +1,106 @@
+<style>
+canvas {
+ width: 10px;
+ height: 10px;
+}
+</style>
+<script>
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+}
+
+const WIDTH = 10;
+const HEIGHT = 10;
+
+const COLORS = [
+ [0, 0, 1, 1],
+ [0, 1, 1, 1],
+ [0, 0.5, 0, 1],
+ [1, 0, 1, 1],
+ [1, 1, 0.5, 1],
+ [0.5, 0, 1, 1],
+ [0.5, 0.5, 0.5, 1],
+ [0.5, 0.5, 1, 1],
+ [0.25, 0, 1, 1],
+ [0, 1, 0, 1]
+];
+
+let canvas;
+let gl;
+let currentFrame = 0;
+
+function step1() {
+
+ // Step 1. Create a WebGL canvas and render into it.
+ // We do this for 10 frames, each time a different color ending in pure green (0, 1, 0, 1).
+
+ canvas = document.querySelector("canvas");
+ canvas.width = WIDTH;
+ canvas.height = HEIGHT;
+
+ gl = canvas.getContext("webgl");
+
+ function clearToColor() {
+ if (currentFrame >= COLORS.length) {
+ requestAnimationFrame(step2);
+ return;
+ }
+
+ gl.clearColor(...COLORS[currentFrame]);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+
+ currentFrame++;
+ requestAnimationFrame(clearToColor);
+ }
+
+ clearToColor();
+}
+
+function step2() {
+
+ // Step 2. Create a framebuffer and render into it, but don't draw into the canvas.");
+
+ const texture = gl.createTexture();
+ gl.bindTexture(gl.TEXTURE_2D, texture);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_T, gl.CLAMP_TO_EDGE);
+ gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, WIDTH, HEIGHT, 0, gl.RGBA, gl.UNSIGNED_BYTE, null);
+
+ const framebuffer = gl.createFramebuffer();
+ gl.bindFramebuffer(gl.FRAMEBUFFER, framebuffer);
+ gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, texture, 0);
+
+ // Clear framebuffer to red (1, 0, 0, 1). This color was not used above.
+
+ gl.clearColor(1, 0, 0, 1);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+
+ gl.bindFramebuffer(gl.FRAMEBUFFER, null);
+
+ // Step 3. Remove the canvas from its parent.
+
+ canvas.parentNode.removeChild(canvas);
+
+ // Step 4. Insert the canvas into a new parent.
+
+ document.getElementById("container2").appendChild(canvas);
+
+ // End result should be a green canvas.
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+
+function runTest() {
+ requestAnimationFrame(step1);
+}
+
+window.addEventListener("load", runTest, false);
+</script>
+<div id="container1">
+<canvas></canvas>
+</div>
+<div id="container2">
+</div>
Property changes on: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document-while-clean.html
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Added: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document.html (0 => 266443)
--- trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document.html 2020-09-02 01:46:53 UTC (rev 266443)
@@ -0,0 +1,39 @@
+<style>
+canvas {
+ width: 10px;
+ height: 10px;
+}
+</style>
+<script>
+function runTest() {
+ // Step 1. Grab the WebGL canvas and render a solid color.
+
+ const canvas = document.querySelector("canvas");
+ canvas.width = 10;
+ canvas.height = 10;
+
+ const gl = canvas.getContext("webgl");
+
+ // Clear to green
+
+ gl.clearColor(0, 1, 0, 1);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+
+ // Step 2. Remove the canvas from its parent.
+
+ canvas.parentNode.removeChild(canvas);
+
+ // Step 3. Insert the canvas into a new parent.
+
+ document.getElementById("container2").appendChild(canvas);
+
+ // End result should be a green canvas.
+}
+
+window.addEventListener("load", runTest, false);
+</script>
+<div id="container1">
+<canvas></canvas>
+</div>
+<div id="container2">
+</div>
Property changes on: trunk/LayoutTests/fast/canvas/webgl/move-canvas-in-document.html
___________________________________________________________________
Added: svn:eol-style
+native
\ No newline at end of property
Added: svn:keywords
+Date Revision
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Modified: trunk/Source/WebCore/ChangeLog (266442 => 266443)
--- trunk/Source/WebCore/ChangeLog 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/ChangeLog 2020-09-02 01:46:53 UTC (rev 266443)
@@ -1,3 +1,61 @@
+2020-09-01 Dean Jackson <[email protected]>
+
+ REGRESSION(r262366): MotionMark1.1 | macOS | Some devices | 1-3% overall regression
+ https://bugs.webkit.org/show_bug.cgi?id=215989
+ <rdar://problem/66845937>
+
+ Reviewed by Darin Adler.
+
+ The new approach to compositing WebGL caused a slowdown in some
+ canvas performance tests. They were notifying the Document
+ of all drawing commands, even on 2d canvases that didn't need
+ to prepare before compositing.
+
+ The solution is to only add the Document as an observer
+ when necessary. This recovers the performance hit - measured
+ using the Canvas Lines MotionMark test.
+
+ Tests: fast/canvas/webgl/move-canvas-in-document-while-clean.html
+ fast/canvas/webgl/move-canvas-in-document.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::prepareCanvasesForDisplayIfNeeded): Leave a FIXME
+ indicating that we should try to avoid the copyToVector if we
+ can ensure that the prepareForDisplay call will not mutate the HashSet.
+
+ * html/CanvasBase.cpp: Remove some copyToVector calls. It should be
+ fine to iterate over the HashSet for these notification functions.
+ (WebCore::CanvasBase::notifyObserversCanvasChanged):
+ (WebCore::CanvasBase::notifyObserversCanvasResized):
+
+ * html/HTMLCanvasElement.cpp:
+ (WebCore::HTMLCanvasElement::HTMLCanvasElement): Don't add the Document as
+ a canvas observer. Wait until we know it is a context that needs observing.
+ (WebCore::HTMLCanvasElement::createContextWebGL): Now we can add it as an
+ observer.
+ (WebCore::HTMLCanvasElement::didMoveToNewDocument): Swap observation to the new
+ Document.
+ (WebCore::HTMLCanvasElement::insertedIntoAncestor): Ditto, but also be aware that
+ we might be in a "dirty" state, and thus need to immediately tell the new
+ document that the canvas needs preparation.
+ (WebCore::HTMLCanvasElement::removedFromAncestor): Remove the observer.
+ (WebCore::HTMLCanvasElement::needsPreparationForDisplay): Use a virtual function
+ on the context instead of checking the type.
+ (WebCore::HTMLCanvasElement::prepareForDisplay): Ditto.
+
+ * html/canvas/CanvasRenderingContext.h: New virtual functions to avoid type checking
+ in HTMLCanvasElement.
+ (WebCore::CanvasRenderingContext::compositingResultsNeedUpdating const):
+ (WebCore::CanvasRenderingContext::needsPreparationForDisplay const):
+ (WebCore::CanvasRenderingContext::prepareForDisplay):
+
+ * html/canvas/WebGLRenderingContextBase.cpp: Implementations of the virtual functions
+ that tell HTMLCanvasElement that it needs to prepareForDisplay.
+ (WebCore::WebGLRenderingContextBase::markContextChanged):
+ (WebCore::WebGLRenderingContextBase::markContextChangedAndNotifyCanvasObserver):
+ (WebCore::WebGLRenderingContextBase::didComposite):
+ * html/canvas/WebGLRenderingContextBase.h:
+
2020-09-01 Chris Dumez <[email protected]>
Make sure BiquadFilterNode tail is getting processed
Modified: trunk/Source/WebCore/dom/Document.cpp (266442 => 266443)
--- trunk/Source/WebCore/dom/Document.cpp 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/dom/Document.cpp 2020-09-02 01:46:53 UTC (rev 266443)
@@ -8635,6 +8635,11 @@
{
// Some canvas contexts need to do work when rendering has finished but
// before their content is composited.
+
+ // FIXME: Calling prepareForDisplay should not call back into a method
+ // that would mutate our m_canvasesNeedingDisplayPreparation list. It
+ // would be nice if this could be enforced to remove the copyToVector.
+
for (auto* canvas : copyToVector(m_canvasesNeedingDisplayPreparation)) {
// However, if they are not in the document body, then they won't
// be composited and thus don't need preparation. Unfortunately they
Modified: trunk/Source/WebCore/html/CanvasBase.cpp (266442 => 266443)
--- trunk/Source/WebCore/html/CanvasBase.cpp 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/html/CanvasBase.cpp 2020-09-02 01:46:53 UTC (rev 266443)
@@ -136,13 +136,13 @@
void CanvasBase::notifyObserversCanvasChanged(const FloatRect& rect)
{
- for (auto& observer : copyToVector(m_observers))
+ for (auto& observer : m_observers)
observer->canvasChanged(*this, rect);
}
void CanvasBase::notifyObserversCanvasResized()
{
- for (auto& observer : copyToVector(m_observers))
+ for (auto& observer : m_observers)
observer->canvasResized(*this);
}
Modified: trunk/Source/WebCore/html/HTMLCanvasElement.cpp (266442 => 266443)
--- trunk/Source/WebCore/html/HTMLCanvasElement.cpp 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/html/HTMLCanvasElement.cpp 2020-09-02 01:46:53 UTC (rev 266443)
@@ -129,7 +129,6 @@
, ActiveDOMObject(document)
{
ASSERT(hasTagName(canvasTag));
- addObserver(document);
}
Ref<HTMLCanvasElement> HTMLCanvasElement::create(Document& document)
@@ -442,6 +441,10 @@
// adapter when there is an active immersive device.
m_context = WebGLRenderingContextBase::create(*this, attrs, type);
if (m_context) {
+ // This new context needs to be observed by the Document, in order
+ // for it to be correctly preparedForRendering before it is composited.
+ addObserver(document());
+
// Need to make sure a RenderLayer and compositing layer get created for the Canvas.
invalidateStyleAndLayerComposition();
#if ENABLE(WEBXR)
@@ -1036,9 +1039,11 @@
void HTMLCanvasElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument)
{
- oldDocument.clearCanvasPreparation(this);
- removeObserver(oldDocument);
- addObserver(newDocument);
+ if (needsPreparationForDisplay()) {
+ oldDocument.clearCanvasPreparation(this);
+ removeObserver(oldDocument);
+ addObserver(newDocument);
+ }
HTMLElement::didMoveToNewDocument(oldDocument, newDocument);
}
@@ -1045,8 +1050,14 @@
Node::InsertedIntoAncestorResult HTMLCanvasElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
- if (insertionType.connectedToDocument)
- addObserver(parentOfInsertedTree.document());
+ if (needsPreparationForDisplay() && insertionType.connectedToDocument) {
+ auto& document = parentOfInsertedTree.document();
+ addObserver(document);
+ // Drawing commands may have been issued to the canvas before now, so we need to
+ // tell the document if we need preparation.
+ if (m_context && m_context->compositingResultsNeedUpdating())
+ document.canvasChanged(*this, FloatRect { });
+ }
return HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
}
@@ -1053,7 +1064,7 @@
void HTMLCanvasElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
{
- if (removalType.disconnectedFromDocument) {
+ if (needsPreparationForDisplay() && removalType.disconnectedFromDocument) {
oldParentOfRemovedTree.document().clearCanvasPreparation(this);
removeObserver(oldParentOfRemovedTree.document());
}
@@ -1063,11 +1074,7 @@
bool HTMLCanvasElement::needsPreparationForDisplay()
{
-#if ENABLE(WEBGL)
- return is<WebGLRenderingContextBase>(m_context.get());
-#else
- return false;
-#endif
+ return m_context && m_context->needsPreparationForDisplay();
}
void HTMLCanvasElement::prepareForDisplay()
@@ -1075,8 +1082,8 @@
#if ENABLE(WEBGL)
ASSERT(needsPreparationForDisplay());
- if (is<WebGLRenderingContextBase>(m_context.get()))
- downcast<WebGLRenderingContextBase>(m_context.get())->prepareForDisplay();
+ if (m_context)
+ m_context->prepareForDisplay();
#endif
}
Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext.h (266442 => 266443)
--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext.h 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext.h 2020-09-02 01:46:53 UTC (rev 266443)
@@ -77,6 +77,10 @@
bool callTracingActive() const { return m_callTracingActive; }
void setCallTracingActive(bool callTracingActive) { m_callTracingActive = callTracingActive; }
+ virtual bool compositingResultsNeedUpdating() const { return false; }
+ virtual bool needsPreparationForDisplay() const { return false; }
+ virtual void prepareForDisplay() { }
+
protected:
explicit CanvasRenderingContext(CanvasBase&);
bool wouldTaintOrigin(const CanvasPattern*);
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (266442 => 266443)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2020-09-02 01:46:53 UTC (rev 266443)
@@ -1069,6 +1069,7 @@
m_context->markContextChanged();
m_layerCleared = false;
+ m_compositingResultsNeedUpdating = true;
if (auto* canvas = htmlCanvas()) {
RenderBox* renderBox = canvas->renderBox();
@@ -1100,9 +1101,7 @@
if (!canvas)
return;
- RenderBox* renderBox = canvas->renderBox();
- if (renderBox && renderBox->hasAcceleratedCompositing())
- canvas->notifyObserversCanvasChanged(FloatRect(FloatPoint(0, 0), clampedCanvasSize()));
+ canvas->notifyObserversCanvasChanged(FloatRect(FloatPoint(0, 0), clampedCanvasSize()));
}
bool WebGLRenderingContextBase::clearIfComposited(GCGLbitfield mask)
@@ -7827,6 +7826,8 @@
void WebGLRenderingContextBase::didComposite()
{
+ m_compositingResultsNeedUpdating = false;
+
if (UNLIKELY(callTracingActive()))
InspectorInstrumentation::didFinishRecordingCanvasFrame(*this);
}
Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h (266442 => 266443)
--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2020-09-02 01:31:45 UTC (rev 266442)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h 2020-09-02 01:46:53 UTC (rev 266443)
@@ -399,8 +399,6 @@
// Used for testing only, from Internals.
WEBCORE_EXPORT void setFailNextGPUStatusCheck();
- void prepareForDisplay();
-
// GraphicsContextGL::Client
void didComposite() override;
void forceContextLost() override;
@@ -521,6 +519,10 @@
virtual void uncacheDeletedBuffer(const WTF::AbstractLocker&, WebGLBuffer*);
+ bool compositingResultsNeedUpdating() const final { return m_compositingResultsNeedUpdating; }
+ bool needsPreparationForDisplay() const final { return true; }
+ void prepareForDisplay() final;
+
RefPtr<GraphicsContextGLOpenGL> m_context;
RefPtr<WebGLContextGroup> m_contextGroup;
Lock m_objectGraphLock;
@@ -658,6 +660,8 @@
bool m_hasRequestedPolicyResolution { false };
bool isContextLostOrPending();
+ bool m_compositingResultsNeedUpdating { false };
+
// Enabled extension objects.
// FIXME: Move some of these to WebGLRenderingContext, the ones not needed for WebGL2
RefPtr<EXTFragDepth> m_extFragDepth;
