Skip to content

Commit

Permalink
Google Photos will not allow preview of editing in Safari.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259263
<rdar://107687640>

Reviewed by Dean Jackson.

OffscreenCanvas can asynchronously push frames to the layer associated with the HTMLCanvasElement that is being controlled.
This happens using a GraphicsLayerAsyncContentsDisplayDelegate, created by the GraphicsLayer for the HTMLCanvasElement.

If style changes that result in the recreation of the GraphicsLayer, the OffscreenCanvas continues to push frames to the original
delegate, and they get dropped.

This makes ImageBufferPipe always ask the current GraphicsLayer to make sure we have a valid delegate, providing the delegate
that was used last time. If the GraphicsLayer has changed, it can reconfigure the delegate to make sure it's targetting the right layer.

We need to reconfigure the delegate, rather than recreating one, since the delegate owns the 'current' frame and we need to ensure this
is retained. Otherwise compositing wouldn't have a frame until the next time the OffscreenCanvas draws (which could be never).

* LayoutTests/fast/canvas/offscreen-toggle-display-expected.html: Added.
* LayoutTests/fast/canvas/offscreen-toggle-display.html: Added.
* Source/WebCore/platform/graphics/GraphicsLayer.cpp:
(WebCore::GraphicsLayer::createAsyncContentsDisplayDelegate):
* Source/WebCore/platform/graphics/GraphicsLayer.h:
* Source/WebCore/platform/graphics/GraphicsLayerContentsDisplayDelegate.h:
* Source/WebCore/platform/graphics/ImageBufferPipe.cpp:
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::contentsLayerIDForModel const):
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:
* Source/WebCore/platform/graphics/ca/cocoa/GraphicsLayerAsyncContentsDisplayDelegateCocoa.h:
* Source/WebCore/platform/graphics/ca/cocoa/GraphicsLayerAsyncContentsDisplayDelegateCocoa.mm:
(WebCore::GraphicsLayerAsyncContentsDisplayDelegateCocoa::updateGraphicsLayerCA):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm:
(WebKit::GraphicsLayerCARemote::createAsyncContentsDisplayDelegate):

Canonical link: https://commits.webkit.org/266120@main
  • Loading branch information
mattwoodrow committed Jul 18, 2023
1 parent a5aad74 commit cbd73ab
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 13 deletions.
17 changes: 17 additions & 0 deletions LayoutTests/fast/canvas/offscreen-toggle-display-expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<html>
<body>
<canvas id="onscreen" width="200" height="200"></canvas>
<script>
const canvas = document.getElementById('onscreen');

const context = canvas.getContext('2d');

const square = new Path2D();
square.rect(50, 50, 100, 100);
context.fillStyle = 'green';
context.fill(square);

</script>
</body>
</html>
37 changes: 37 additions & 0 deletions LayoutTests/fast/canvas/offscreen-toggle-display.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<html>
<head>
<meta name="fuzzy" content="maxDifference=63; totalPixels=400" />
</head>
<body>
<canvas id="offscreen" width="200" height="200"></canvas>
<script>
const canvas = document.getElementById('offscreen');

const offscreenCanvas = canvas.transferControlToOffscreen();
const offscreenContext = offscreenCanvas.getContext('2d');


if (window.testRunner)
testRunner.waitUntilDone();

requestAnimationFrame(function() {
const square = new Path2D();
square.rect(50, 50, 100, 100);
offscreenContext.fillStyle = 'green';
offscreenContext.fill(square);
offscreenContext.commit();

requestAnimationFrame(function() {
canvas.style.display = "none";

requestAnimationFrame(function() {
canvas.style.display = "inline";
if (window.testRunner)
testRunner.notifyDone();
});
});
});
</script>
</body>
</html>
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/GraphicsLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ void GraphicsLayer::setContentsDisplayDelegate(RefPtr<GraphicsLayerContentsDispl
{
}

RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayer::createAsyncContentsDisplayDelegate()
RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayer::createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate*)
{
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/GraphicsLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ class GraphicsLayer : public RefCounted<GraphicsLayer> {
virtual void setContentsToPlatformLayerHost(LayerHostingContextIdentifier) { }
virtual void setContentsToVideoElement(HTMLVideoElement&, ContentsLayerPurpose) { }
virtual void setContentsDisplayDelegate(RefPtr<GraphicsLayerContentsDisplayDelegate>&&, ContentsLayerPurpose);
WEBCORE_EXPORT virtual RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate();
WEBCORE_EXPORT virtual RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate* existing);
#if ENABLE(MODEL_ELEMENT)
enum class ModelInteraction : uint8_t { Enabled, Disabled };
virtual void setContentsToModel(RefPtr<Model>&&, ModelInteraction) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ class WEBCORE_EXPORT GraphicsLayerContentsDisplayDelegate : public RefCounted<Gr
#endif
};

class WEBCORE_EXPORT GraphicsLayerAsyncContentsDisplayDelegate : public GraphicsLayerContentsDisplayDelegate {
class GraphicsLayerAsyncContentsDisplayDelegate : public GraphicsLayerContentsDisplayDelegate {
public:
virtual ~GraphicsLayerAsyncContentsDisplayDelegate() = default;

virtual bool tryCopyToLayer(ImageBuffer&) = 0;
virtual bool WEBCORE_EXPORT tryCopyToLayer(ImageBuffer&) = 0;

virtual bool isGraphicsLayerAsyncContentsDisplayDelegateCocoa() const { return false; }
virtual bool isGraphicsLayerCARemoteAsyncContentsDisplayDelegate() const { return false; }
};

}
3 changes: 1 addition & 2 deletions Source/WebCore/platform/graphics/ImageBufferPipe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ class ImageBufferPipeDelegate : public ImageBufferPipe {
void setContentsToLayer(GraphicsLayer& layer) final
{
Locker locker { m_source->m_lock };
if (!m_source->m_delegate)
m_source->m_delegate = layer.createAsyncContentsDisplayDelegate();
m_source->m_delegate = layer.createAsyncContentsDisplayDelegate(m_source->m_delegate.get());
}

RefPtr<ImageBufferPipe::Source> source() const final
Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,7 @@ void GraphicsLayerCA::setContentsToPlatformLayer(PlatformLayer* platformLayer, C
m_contentsLayer = WTFMove(platformCALayer);
else
m_contentsLayer = createPlatformCALayer(platformLayer, this);
m_contentsLayer->setBackingStoreAttached(false);
} else
m_contentsLayer = nullptr;

Expand Down Expand Up @@ -5001,8 +5002,12 @@ Vector<std::pair<String, double>> GraphicsLayerCA::acceleratedAnimationsForTesti
return animations;
}

RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayerCA::createAsyncContentsDisplayDelegate()
RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayerCA::createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate* existing)
{
if (existing && existing->isGraphicsLayerAsyncContentsDisplayDelegateCocoa()) {
static_cast<GraphicsLayerAsyncContentsDisplayDelegateCocoa*>(existing)->updateGraphicsLayerCA(*this);
return existing;
}
return adoptRef(new GraphicsLayerAsyncContentsDisplayDelegateCocoa(*this));
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class GraphicsLayerCA : public GraphicsLayer, public PlatformCALayerClient {

constexpr static CompositingCoordinatesOrientation defaultContentsOrientation = CompositingCoordinatesOrientation::TopDown;

WEBCORE_EXPORT RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate() override;
WEBCORE_EXPORT RefPtr<GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate*) override;

#if ENABLE(THREADED_ANIMATION_RESOLUTION)
WEBCORE_EXPORT void setAcceleratedEffectsAndBaseValues(AcceleratedEffects&&, AcceleratedEffectValues&&) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ class GraphicsLayerAsyncContentsDisplayDelegateCocoa : public GraphicsLayerAsync
bool tryCopyToLayer(ImageBuffer&) final;
void display(PlatformCALayer&) final { }

bool isGraphicsLayerAsyncContentsDisplayDelegateCocoa() const final { return true; }

void updateGraphicsLayerCA(GraphicsLayerCA&);

private:
RetainPtr<CALayer> m_layer;
RefPtr<NativeImage> m_image;
};

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,23 @@

bool GraphicsLayerAsyncContentsDisplayDelegateCocoa::tryCopyToLayer(ImageBuffer& image)
{
auto native = image.clone()->sinkIntoNativeImage();
m_image = image.clone()->sinkIntoNativeImage();

[CATransaction begin];
[CATransaction setDisableActions:YES];

[m_layer setContents:(__bridge id)native->platformImage().get()];
[m_layer setContents:(__bridge id)m_image->platformImage().get()];

[CATransaction commit];

return true;
}

void GraphicsLayerAsyncContentsDisplayDelegateCocoa::updateGraphicsLayerCA(GraphicsLayerCA& layer)
{
layer.setContentsToPlatformLayer(m_layer.get(), GraphicsLayer::ContentsLayerPurpose::Canvas);
if (m_image)
[m_layer setContents:(__bridge id)m_image->platformImage().get()];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA {

WebCore::Color pageTiledBackingBorderColor() const override;

RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate() final;
RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate(WebCore::GraphicsLayerAsyncContentsDisplayDelegate*) final;

RemoteLayerTreeContext* m_context;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
m_layerID = layerID;
}

bool isGraphicsLayerCARemoteAsyncContentsDisplayDelegate() const final { return true; }

private:
Ref<IPC::Connection> m_connection;
DrawingAreaIdentifier m_drawingArea;
Expand All @@ -172,12 +174,19 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
WebCore::RenderingResourceIdentifier m_surfaceIdentifier WTF_GUARDED_BY_LOCK(m_surfaceLock);
};

RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayerCARemote::createAsyncContentsDisplayDelegate()
RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayerCARemote::createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate* existing)
{
if (!m_context || !m_context->drawingAreaIdentifier() || !WebProcess::singleton().parentProcessConnection())
return nullptr;

auto delegate = adoptRef(new GraphicsLayerCARemoteAsyncContentsDisplayDelegate(*WebProcess::singleton().parentProcessConnection(), m_context->drawingAreaIdentifier()));
RefPtr<GraphicsLayerCARemoteAsyncContentsDisplayDelegate> delegate;
if (existing && existing->isGraphicsLayerCARemoteAsyncContentsDisplayDelegate())
delegate = static_cast<GraphicsLayerCARemoteAsyncContentsDisplayDelegate*>(existing);

if (!delegate) {
ASSERT(!existing);
delegate = adoptRef(new GraphicsLayerCARemoteAsyncContentsDisplayDelegate(*WebProcess::singleton().parentProcessConnection(), m_context->drawingAreaIdentifier()));
}

auto layerID = setContentsToAsyncDisplayDelegate(delegate, ContentsLayerPurpose::Canvas);

Expand Down

0 comments on commit cbd73ab

Please sign in to comment.