Capturing a canvas that is not in the DOM can lead to erratic frame r…

…ates or no frame emission at all https://bugs.webkit.org/show_bug.cgi?id=240380 Patch by Dan Glastonbury <djg@apple.com> on 2022-05-25 Reviewed by Simon Fraser. * Source/WebCore/dom/Document.cpp: (WebCore::Document::canvasChanged): Schedule a rendering update whenever a canvas with out a rect needing display preparation is added. This ensures that the prepareForDisplay is called on all pending canvases, since this is handled in doAfterUpdateRendering. * Source/WebCore/dom/Document.h: Update size of RenderingUpdateState to accomodate PrepareCanvasesForDisplay. * Source/WebCore/page/Page.cpp: * Source/WebCore/page/Page.h: Introduce new RenderingUpdateStep, PrepareCanvasesForDisplay, which signals that prepareCanvasesForDisplayInNeeded() needs to be called from doAfterRenderingUpdate(). Update size of RenderingUpdateState to accomodate PrepareCanvasesForDisplay. * LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas-expected.txt: Added. * LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas.html: Test that frame rate of captured video from out-of-DOM canvas is with in 25% of the generating frame rate. Canonical link: https://commits.webkit.org/250996@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294864 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 26, 2022 · 994751c42e3184931d43481820724f17d77f23ff · 994751c
1 parent abe1582
commit 994751c42e3184931d43481820724f17d77f23ff
Showing 7 changed files with 68 additions and 4 deletions.
diff --git a/...t/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas-expected.txt b/...t/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas-expected.txt
@@ -0,0 +1,4 @@
+
+
+PASS check frame of capture canvas is sufficient
+
diff --git a/...ests/fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas.html b/...ests/fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Capture canvas to video track framerate</title>
+    <script src="../../resources/testharness.js"></script>
+    <script src="../../resources/testharnessreport.js"></script>
+    <script src="../../webrtc/routines.js"></script>
+</head>
+<body>
+    <video id="video" autoplay playsInline controls width="200px"></video>
+    <script>
+promise_test(async (t) => {
+    const canvas = document.createElement('canvas');
+    canvas.width = 200;
+    canvas.height = 200;
+
+    const ctx = canvas.getContext('2d');
+
+    let frames = 0;
+    const loop = () => {
+        frames += 1;
+        ctx.fillStyle = (ctx.fillStyle !== '#ff0000') ? 'red' : 'green';
+        ctx.fillRect(0, 0, canvas.width, canvas.height);
+        setTimeout(loop, 1000/30);
+    }
+
+    loop();
+
+    const stream = canvas.captureStream();
+    video.srcObject = stream;
+    video.play();
+
+    const frameRate = await computeFrameRate(stream, video);
+
+    // If the test was unable to generate any frames then nothing meaningful can be determined. Our test only makes sense if we have sufficient fps.
+    if (frames <= 10)
+        return;
+
+    // Check that the difference in expected and observed frame rates is < 25%
+    const percentDiff = Math.abs(frameRate - frames) / frames * 100;
+    assert_less_than(percentDiff, 25, `frame rate difference between ${frames} & ${frameRate} is below 25%`);
+}, "check frame of capture canvas is sufficient");
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -3704,6 +3704,8 @@ fast/events/contextmenu-lookup-action-for-image.html [ Failure ]
 fast/events/webkit-media-key-events-constructor.html [ Failure ]
 fast/forms/validation-message-maxLength.html [ Failure ]
 fast/mediacapturefromelement/CanvasCaptureMediaStream-2d-events.html [ Failure ]
+# canvas.captureStream is not a function
+webkit.org/b/240380 fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-canvas.html [ Failure ]
 fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html [ Failure ]
 fast/mediacapturefromelement/CanvasCaptureMediaStream-clone-track.html [ Failure ]
 fast/mediacapturefromelement/CanvasCaptureMediaStream-creation.html [ Failure ]

diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
@@ -9095,13 +9095,20 @@ void Document::clearCanvasPreparation(HTMLCanvasElement& canvas)
     m_canvasesNeedingDisplayPreparation.remove(canvas);
 }
 
-void Document::canvasChanged(CanvasBase& canvasBase, const std::optional<FloatRect>&)
+void Document::canvasChanged(CanvasBase& canvasBase, const std::optional<FloatRect>& changedRect)
 {
     if (!is<HTMLCanvasElement>(canvasBase))
         return;
     auto& canvas = downcast<HTMLCanvasElement>(canvasBase);
-    if (canvas.needsPreparationForDisplay())
+    if (canvas.needsPreparationForDisplay()) {
         m_canvasesNeedingDisplayPreparation.add(canvas);
+        // Schedule a rendering update to force handling of prepareForDisplay
+        // for any queued canvases. This is especially important for any canvas
+        // that is not in the DOM, as those don't have a rect to invalidate to
+        // trigger an update. <http://bugs.webkit.org/show_bug.cgi?id=240380>.
+        if (!changedRect)
+            scheduleRenderingUpdate(RenderingUpdateStep::PrepareCanvasesForDisplay);
+    }
 }
 
 void Document::canvasDestroyed(CanvasBase& canvasBase)

diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
@@ -265,7 +265,7 @@ enum class MediaProducerMediaCaptureKind : uint8_t;
 enum class MediaProducerMutedState : uint8_t;
 enum class RouteSharingPolicy : uint8_t;
 enum class ShouldOpenExternalURLsPolicy : uint8_t;
-enum class RenderingUpdateStep : uint16_t;
+enum class RenderingUpdateStep : uint32_t;
 enum class StyleColorOptions : uint8_t;
 enum class MutationObserverOptionType : uint8_t;
 

diff --git a/Source/WebCore/page/Page.cpp b/Source/WebCore/page/Page.cpp
@@ -1793,6 +1793,8 @@ void Page::doAfterUpdateRendering()
 
     DebugPageOverlays::doAfterUpdateRendering(*this);
 
+    m_renderingUpdateRemainingSteps.last().remove(RenderingUpdateStep::PrepareCanvasesForDisplay);
+
     forEachDocument([] (Document& document) {
         document.prepareCanvasesForDisplayIfNeeded();
     });
@@ -3836,6 +3838,7 @@ WTF::TextStream& operator<<(WTF::TextStream& ts, RenderingUpdateStep step)
     case RenderingUpdateStep::ScrollingTreeUpdate: ts << "ScrollingTreeUpdate"; break;
 #endif
     case RenderingUpdateStep::VideoFrameCallbacks: ts << "VideoFrameCallbacks"; break;
+    case RenderingUpdateStep::PrepareCanvasesForDisplay: ts << "PrepareCanvasesForDisplay"; break;
     }
     return ts;
 }

diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
@@ -208,7 +208,7 @@ enum class FinalizeRenderingUpdateFlags : uint8_t {
     InvalidateImagesWithAsyncDecodes    = 1 << 1,
 };
 
-enum class RenderingUpdateStep : uint16_t {
+enum class RenderingUpdateStep : uint32_t {
     Resize                          = 1 << 0,
     Scroll                          = 1 << 1,
     MediaQueryEvaluation            = 1 << 2,
@@ -227,6 +227,7 @@ enum class RenderingUpdateStep : uint16_t {
 #endif
     FlushAutofocusCandidates        = 1 << 14,
     VideoFrameCallbacks             = 1 << 15,
+    PrepareCanvasesForDisplay       = 1 << 16,
 };
 
 constexpr OptionSet<RenderingUpdateStep> updateRenderingSteps = {
@@ -243,6 +244,7 @@ constexpr OptionSet<RenderingUpdateStep> updateRenderingSteps = {
     RenderingUpdateStep::WheelEventMonitorCallbacks,
     RenderingUpdateStep::CursorUpdate,
     RenderingUpdateStep::EventRegionUpdate,
+    RenderingUpdateStep::PrepareCanvasesForDisplay,
 };
 
 constexpr auto allRenderingUpdateSteps = updateRenderingSteps | OptionSet<RenderingUpdateStep> {