- Revision
- 284828
- Author
- [email protected]
- Date
- 2021-10-25 14:50:49 -0700 (Mon, 25 Oct 2021)
Log Message
Cherry-pick r283489. rdar://problem/84630680
Regression (r283238)[ MacOS wk1 ] fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html is timing out
https://bugs.webkit.org/show_bug.cgi?id=231022
Patch by Kimmo Kinnunen <[email protected]> on 2021-10-04
Reviewed by Youenn Fablet.
Originally the implementation would always return red frame, and the test would pass.
r283238 changed the implementation to not return a sample if there is not a display buffer,
as logically such cannot be used as a sample.
This broke the test case since the CanvasCaptureMediaStreamTrack would try to capture
the canvas display buffer during next runloop iteration (0s timeout) after each modification.
This does not work, as the display buffer is composed during "prepare for display"
phase.
Add CanvasBase observers to observe that display buffer has been prepared, and capture
the media sample after that observer has fired.
The test would work for wk2 due to timing related differences, preparation would have
typically run before the canvas capture 0s timeout.
Fixes fast/mediastream/captureStream/canvas3d.html for wk1.
* Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:
(WebCore::CanvasCaptureMediaStreamTrack::Source::startProducingData):
(WebCore::CanvasCaptureMediaStreamTrack::Source::canvasChanged):
(WebCore::CanvasCaptureMediaStreamTrack::Source::canvasDisplayBufferPrepared):
* Modules/mediastream/CanvasCaptureMediaStreamTrack.h:
* html/CanvasBase.cpp:
(WebCore::CanvasBase::addDisplayBufferObserver):
(WebCore::CanvasBase::removeDisplayBufferObserver):
(WebCore::CanvasBase::notifyObserversCanvasDisplayBufferPrepared):
* html/CanvasBase.h:
(WebCore::CanvasBase::hasDisplayBufferObservers const):
* html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::prepareForDisplay):
Move the "prepare only when the owner element is in the tree" logic to
its correct place to the caller, e.g. to the element itself.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283489 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebCore/ChangeLog (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/ChangeLog 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/ChangeLog 2021-10-25 21:50:49 UTC (rev 284828)
@@ -1,5 +1,88 @@
2021-10-25 Null <[email protected]>
+ Cherry-pick r283489. rdar://problem/84630680
+
+ Regression (r283238)[ MacOS wk1 ] fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html is timing out
+ https://bugs.webkit.org/show_bug.cgi?id=231022
+
+ Patch by Kimmo Kinnunen <[email protected]> on 2021-10-04
+ Reviewed by Youenn Fablet.
+
+ Originally the implementation would always return red frame, and the test would pass.
+ r283238 changed the implementation to not return a sample if there is not a display buffer,
+ as logically such cannot be used as a sample.
+ This broke the test case since the CanvasCaptureMediaStreamTrack would try to capture
+ the canvas display buffer during next runloop iteration (0s timeout) after each modification.
+ This does not work, as the display buffer is composed during "prepare for display"
+ phase.
+
+ Add CanvasBase observers to observe that display buffer has been prepared, and capture
+ the media sample after that observer has fired.
+
+ The test would work for wk2 due to timing related differences, preparation would have
+ typically run before the canvas capture 0s timeout.
+
+ Fixes fast/mediastream/captureStream/canvas3d.html for wk1.
+
+ * Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::startProducingData):
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::canvasChanged):
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::canvasDisplayBufferPrepared):
+ * Modules/mediastream/CanvasCaptureMediaStreamTrack.h:
+ * html/CanvasBase.cpp:
+ (WebCore::CanvasBase::addDisplayBufferObserver):
+ (WebCore::CanvasBase::removeDisplayBufferObserver):
+ (WebCore::CanvasBase::notifyObserversCanvasDisplayBufferPrepared):
+ * html/CanvasBase.h:
+ (WebCore::CanvasBase::hasDisplayBufferObservers const):
+ * html/canvas/WebGLRenderingContextBase.cpp:
+ (WebCore::WebGLRenderingContextBase::prepareForDisplay):
+ Move the "prepare only when the owner element is in the tree" logic to
+ its correct place to the caller, e.g. to the element itself.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283489 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-10-04 Kimmo Kinnunen <[email protected]>
+
+ Regression (r283238)[ MacOS wk1 ] fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html is timing out
+ https://bugs.webkit.org/show_bug.cgi?id=231022
+
+ Reviewed by Youenn Fablet.
+
+ Originally the implementation would always return red frame, and the test would pass.
+ r283238 changed the implementation to not return a sample if there is not a display buffer,
+ as logically such cannot be used as a sample.
+ This broke the test case since the CanvasCaptureMediaStreamTrack would try to capture
+ the canvas display buffer during next runloop iteration (0s timeout) after each modification.
+ This does not work, as the display buffer is composed during "prepare for display"
+ phase.
+
+ Add CanvasBase observers to observe that display buffer has been prepared, and capture
+ the media sample after that observer has fired.
+
+ The test would work for wk2 due to timing related differences, preparation would have
+ typically run before the canvas capture 0s timeout.
+
+ Fixes fast/mediastream/captureStream/canvas3d.html for wk1.
+
+ * Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::startProducingData):
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::canvasChanged):
+ (WebCore::CanvasCaptureMediaStreamTrack::Source::canvasDisplayBufferPrepared):
+ * Modules/mediastream/CanvasCaptureMediaStreamTrack.h:
+ * html/CanvasBase.cpp:
+ (WebCore::CanvasBase::addDisplayBufferObserver):
+ (WebCore::CanvasBase::removeDisplayBufferObserver):
+ (WebCore::CanvasBase::notifyObserversCanvasDisplayBufferPrepared):
+ * html/CanvasBase.h:
+ (WebCore::CanvasBase::hasDisplayBufferObservers const):
+ * html/canvas/WebGLRenderingContextBase.cpp:
+ (WebCore::WebGLRenderingContextBase::prepareForDisplay):
+ Move the "prepare only when the owner element is in the tree" logic to
+ its correct place to the caller, e.g. to the element itself.
+
+2021-10-25 Null <[email protected]>
+
Cherry-pick r283299. rdar://problem/84629227
Cocoa GraphicsContextGLOpenGL should be more robust in destruction
Modified: branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp 2021-10-25 21:50:49 UTC (rev 284828)
@@ -89,6 +89,7 @@
if (!m_canvas)
return;
m_canvas->addObserver(*this);
+ m_canvas->addDisplayBufferObserver(*this);
if (!m_frameRequestRate)
return;
@@ -104,6 +105,7 @@
if (!m_canvas)
return;
m_canvas->removeObserver(*this);
+ m_canvas->removeDisplayBufferObserver(*this);
}
void CanvasCaptureMediaStreamTrack::Source::requestFrameTimerFired()
@@ -152,7 +154,13 @@
void CanvasCaptureMediaStreamTrack::Source::canvasChanged(CanvasBase& canvas, const std::optional<FloatRect>&)
{
ASSERT_UNUSED(canvas, m_canvas == &canvas);
+ if (m_canvas->renderingContext() && m_canvas->renderingContext()->needsPreparationForDisplay())
+ return;
+ scheduleCaptureCanvas();
+}
+void CanvasCaptureMediaStreamTrack::Source::scheduleCaptureCanvas()
+{
// FIXME: We should try to generate the frame at the time the screen is being updated.
if (m_captureCanvasTimer.isActive())
return;
@@ -159,6 +167,15 @@
m_captureCanvasTimer.startOneShot(0_s);
}
+void CanvasCaptureMediaStreamTrack::Source::canvasDisplayBufferPrepared(CanvasBase& canvas)
+{
+ ASSERT_UNUSED(canvas, m_canvas == &canvas);
+ // FIXME: Here we should capture the image instead.
+ // However, submitting the sample to the receiver might cause layout,
+ // and currently the display preparation is done after layout.
+ scheduleCaptureCanvas();
+}
+
void CanvasCaptureMediaStreamTrack::Source::captureCanvas()
{
ASSERT(m_canvas);
Modified: branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.h (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.h 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.h 2021-10-25 21:50:49 UTC (rev 284828)
@@ -30,6 +30,7 @@
#include "MediaStreamTrack.h"
#include "Timer.h"
#include <wtf/TypeCasts.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -50,7 +51,7 @@
private:
const char* activeDOMObjectName() const override;
- class Source final : public RealtimeMediaSource, private CanvasObserver {
+ class Source final : public RealtimeMediaSource, private CanvasObserver, private CanvasDisplayBufferObserver {
public:
static Ref<Source> create(HTMLCanvasElement&, std::optional<double>&& frameRequestRate);
@@ -60,18 +61,21 @@
private:
Source(HTMLCanvasElement&, std::optional<double>&&);
- // CanvasObserver API
+ // CanvasObserver overrides.
void canvasChanged(CanvasBase&, const std::optional<FloatRect>&) final;
void canvasResized(CanvasBase&) final;
void canvasDestroyed(CanvasBase&) final;
- // RealtimeMediaSource API
+ // CanvasDisplayBufferObserver overrides.
+ void canvasDisplayBufferPrepared(CanvasBase&) final;
+
+ // RealtimeMediaSource overrides.
void startProducingData() final;
void stopProducingData() final;
const RealtimeMediaSourceCapabilities& capabilities() final { return RealtimeMediaSourceCapabilities::emptyCapabilities(); }
const RealtimeMediaSourceSettings& settings() final;
void settingsDidChange(OptionSet<RealtimeMediaSourceSettings::Flag>) final;
-
+ void scheduleCaptureCanvas();
void captureCanvas();
void requestFrameTimerFired();
Modified: branches/safari-612-branch/Source/WebCore/html/CanvasBase.cpp (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/html/CanvasBase.cpp 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/html/CanvasBase.cpp 2021-10-25 21:50:49 UTC (rev 284828)
@@ -162,6 +162,22 @@
#endif
}
+void CanvasBase::addDisplayBufferObserver(CanvasDisplayBufferObserver& observer)
+{
+ m_displayBufferObservers.add(&observer);
+}
+
+void CanvasBase::removeDisplayBufferObserver(CanvasDisplayBufferObserver& observer)
+{
+ m_displayBufferObservers.remove(observer);
+}
+
+void CanvasBase::notifyObserversCanvasDisplayBufferPrepared()
+{
+ for (auto& observer : m_displayBufferObservers)
+ observer.canvasDisplayBufferPrepared(*this);
+}
+
HashSet<Element*> CanvasBase::cssCanvasClients() const
{
HashSet<Element*> cssCanvasClients;
Modified: branches/safari-612-branch/Source/WebCore/html/CanvasBase.h (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/html/CanvasBase.h 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/html/CanvasBase.h 2021-10-25 21:50:49 UTC (rev 284828)
@@ -28,6 +28,7 @@
#include "IntSize.h"
#include <wtf/HashSet.h>
#include <wtf/TypeCasts.h>
+#include <wtf/WeakHashSet.h>
namespace WebCore {
@@ -54,6 +55,13 @@
virtual void canvasDestroyed(CanvasBase&) = 0;
};
+class CanvasDisplayBufferObserver : public CanMakeWeakPtr<CanvasDisplayBufferObserver> {
+public:
+ virtual ~CanvasDisplayBufferObserver() = default;
+
+ virtual void canvasDisplayBufferPrepared(CanvasBase&) = 0;
+};
+
class CanvasBase {
public:
virtual ~CanvasBase();
@@ -92,6 +100,10 @@
void notifyObserversCanvasChanged(const std::optional<FloatRect>&);
void notifyObserversCanvasResized();
void notifyObserversCanvasDestroyed(); // Must be called in destruction before clearing m_context.
+ void addDisplayBufferObserver(CanvasDisplayBufferObserver&);
+ void removeDisplayBufferObserver(CanvasDisplayBufferObserver&);
+ void notifyObserversCanvasDisplayBufferPrepared();
+ bool hasDisplayBufferObservers() const { return !m_displayBufferObservers.computesEmpty(); }
HashSet<Element*> cssCanvasClients() const;
@@ -132,6 +144,7 @@
bool m_didNotifyObserversCanvasDestroyed { false };
#endif
HashSet<CanvasObserver*> m_observers;
+ WeakHashSet<CanvasDisplayBufferObserver> m_displayBufferObservers;
};
} // namespace WebCore
Modified: branches/safari-612-branch/Source/WebCore/html/HTMLCanvasElement.cpp (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/html/HTMLCanvasElement.cpp 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/html/HTMLCanvasElement.cpp 2021-10-25 21:50:49 UTC (rev 284828)
@@ -1054,8 +1054,23 @@
{
ASSERT(needsPreparationForDisplay());
+ bool shouldPrepare = true;
+#if ENABLE(WEBGL)
+ // FIXME: Currently the below prepare skip logic is conservative and applies only to
+ // WebGL elements.
+ if (is<WebGLRenderingContextBase>(m_context)) {
+ // If the canvas is not in the document body, then it won't be
+ // composited and thus doesn't need preparation. Unfortunately
+ // it can't tell at the time it was added to the list, since it
+ // could be inserted or removed from the document body afterwards.
+ shouldPrepare = isInTreeScope() || hasDisplayBufferObservers();
+ }
+#endif
+ if (!shouldPrepare)
+ return;
if (m_context)
m_context->prepareForDisplay();
+ notifyObserversCanvasDisplayBufferPrepared();
}
bool HTMLCanvasElement::isControlledByOffscreen() const
Modified: branches/safari-612-branch/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (284827 => 284828)
--- branches/safari-612-branch/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2021-10-25 21:50:46 UTC (rev 284827)
+++ branches/safari-612-branch/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp 2021-10-25 21:50:49 UTC (rev 284828)
@@ -8016,14 +8016,6 @@
if (!m_context)
return;
- // If the canvas is not in the document body, then it won't be
- // composited and thus doesn't need preparation. Unfortunately
- // it can't tell at the time it was added to the list, since it
- // could be inserted or removed from the document body afterwards.
- auto canvas = htmlCanvas();
- if (!canvas || !canvas->isInTreeScope())
- return;
-
m_context->prepareForDisplay();
}
