Skip to content

Commit

Permalink
visionOS: Text jumps by subpixel increments when repainting
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261472
rdar://109372324

Reviewed by Mike Wyrzykowski, Richard Robinson and Dean Jackson.

* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.h:
* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm:
(WebKit::GraphicsContextCGDisplayList::GraphicsContextCGDisplayList):
(WebKit::CGDisplayListImageBufferBackend::context):
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::paintContents):
(WebKit::RemoteLayerBackingStore::drawInContext):
(WebKit::RemoteLayerBackingStoreProperties::applyBackingStoreToLayer):
Now that it is possible to specify the resolution at which a dynamic content
scaling display list is recorded, plumb the backing store scale to that mechanism,
and remove all of our workarounds from the days when it was only recorded at 1x.

This fixes a bug where the integer rounding of the backing store size coupled
with subpixel CTMs would make text quantization cause glyphs to land at different
quantized positions between the 2x tiles and the 1x recorded display list.

Canonical link: https://commits.webkit.org/268107@main
  • Loading branch information
hortont424 committed Sep 19, 2023
1 parent e49ec88 commit 7eb6ed0
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class CGDisplayListImageBufferBackend : public WebCore::ImageBufferCGBackend, pu
// ImageBufferBackendSharing
ImageBufferBackendSharing* toBackendSharing() final { return this; }

mutable std::unique_ptr<WebCore::GraphicsContext> m_context;
mutable std::unique_ptr<WebCore::GraphicsContextCG> m_context;
RetainPtr<id> m_resourceCache;
WebCore::RenderingMode m_renderingMode;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,38 +57,14 @@ static CFDictionaryRef makeContextOptions(const CGDisplayListImageBufferBackend:

class GraphicsContextCGDisplayList : public WebCore::GraphicsContextCG {
public:
GraphicsContextCGDisplayList(const CGDisplayListImageBufferBackend::Parameters& parameters, WebCore::RenderingMode renderingMode)
: GraphicsContextCG(adoptCF(WKCGCommandsContextCreate(parameters.logicalSize, makeContextOptions(parameters))).autorelease(), GraphicsContextCG::Unknown, renderingMode)
GraphicsContextCGDisplayList(const CGDisplayListImageBufferBackend::Parameters& parameters, WebCore::IntSize backendSize, WebCore::RenderingMode renderingMode)
: GraphicsContextCG(adoptCF(WKCGCommandsContextCreate(backendSize, makeContextOptions(parameters))).autorelease(), GraphicsContextCG::Unknown, renderingMode)
{
m_immutableBaseTransform.scale(parameters.resolutionScale);
m_inverseImmutableBaseTransform = *m_immutableBaseTransform.inverse();
m_resolutionScale = parameters.resolutionScale;
}

void setCTM(const WebCore::AffineTransform& transform) final
{
GraphicsContextCG::setCTM(m_inverseImmutableBaseTransform * transform);
}

WebCore::AffineTransform getCTM(IncludeDeviceScale includeDeviceScale) const final
{
return m_immutableBaseTransform * GraphicsContextCG::getCTM(includeDeviceScale);
}

std::optional<std::pair<float, float>> scaleForRoundingToDevicePixels() const final
{
auto scale = WebCore::GraphicsContextCG::scaleForRoundingToDevicePixels().value_or(std::make_pair<float>(1, 1));
return { { scale.first * m_resolutionScale, scale.second * m_resolutionScale } };
}

bool canUseShadowBlur() const final { return false; }

bool needsCachedNativeImageInvalidationWorkaround(WebCore::RenderingMode) override { return true; }

private:
WebCore::AffineTransform m_immutableBaseTransform;
WebCore::AffineTransform m_inverseImmutableBaseTransform;
float m_resolutionScale;
};

WTF_MAKE_ISO_ALLOCATED_IMPL(CGDisplayListImageBufferBackend);
Expand Down Expand Up @@ -152,8 +128,10 @@ void setCTM(const WebCore::AffineTransform& transform) final

WebCore::GraphicsContext& CGDisplayListImageBufferBackend::context()
{
if (!m_context)
m_context = makeUnique<GraphicsContextCGDisplayList>(m_parameters, m_renderingMode);
if (!m_context) {
m_context = makeUnique<GraphicsContextCGDisplayList>(m_parameters, backendSize(), m_renderingMode);
applyBaseTransform(*m_context);
}
return *m_context;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class RemoteLayerBackingStore : public CanMakeWeakPtr<RemoteLayerBackingStore> {
private:
RemoteLayerBackingStoreCollection* backingStoreCollection() const;

void drawInContext(WebCore::GraphicsContext&, WTF::Function<void()>&& additionalContextSetupCallback = nullptr);
void drawInContext(WebCore::GraphicsContext&);

struct Buffer {
RefPtr<WebCore::ImageBuffer> imageBuffer;
Expand Down
11 changes: 3 additions & 8 deletions Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -570,10 +570,7 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
auto& displayListContext = m_displayListBuffer->context();

BifurcatedGraphicsContext context(m_frontBuffer.imageBuffer->context(), displayListContext);
drawInContext(context, [&] {
displayListContext.scale(FloatSize(1, -1));
displayListContext.translate(0, -m_parameters.size.height());
});
drawInContext(context);
return;
}
#endif
Expand All @@ -582,16 +579,13 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
drawInContext(context);
}

void RemoteLayerBackingStore::drawInContext(GraphicsContext& context, WTF::Function<void()>&& additionalContextSetupCallback)
void RemoteLayerBackingStore::drawInContext(GraphicsContext& context)
{
auto markFrontBufferNotCleared = makeScopeExit([&]() {
m_frontBuffer.isCleared = false;
});
GraphicsContextStateSaver stateSaver(context);

if (additionalContextSetupCallback)
additionalContextSetupCallback();

// If we have less than webLayerMaxRectsToPaint rects to paint and they cover less
// than webLayerWastedSpaceThreshold of the total dirty area, we'll repaint each rect separately.
// Otherwise, repaint the entire bounding box of the dirty region.
Expand Down Expand Up @@ -770,6 +764,7 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
if (!replayCGDisplayListsIntoBackingStore) {
[layer setValue:@1 forKeyPath:WKCGDisplayListEnabledKey];
[layer setValue:@1 forKeyPath:WKCGDisplayListBifurcationEnabledKey];
[layer setValue:@(layer.contentsScale) forKeyPath:@"separatedOptions.bifurcationScale"];
} else
layer.opaque = m_isOpaque;
[(WKCompositingLayer *)layer _setWKContents:contents.get() withDisplayList:WTFMove(std::get<CGDisplayList>(*m_displayListBufferHandle)) replayForTesting:replayCGDisplayListsIntoBackingStore];
Expand Down

0 comments on commit 7eb6ed0

Please sign in to comment.