Skip to content

Commit

Permalink
[UnifiedPDF] Highlighted text in PDF disappears momentarily when Safa…
Browse files Browse the repository at this point in the history
…ri is resized and zoomed

https://bugs.webkit.org/show_bug.cgi?id=273523
rdar://125426369

Reviewed by Simon Fraser.

Our current selection drawing strategy is to hand off a graphics context
to the `-[PDFSelection drawForPage:withBox:active:inContext:]` API,
following which PDFKit duly manipulates the context with selection
geometry. Unfortunately, PDFKit uses the "multiply" compositing mode when
filling the selection paths in our supplied context. This can result in
the white flashing described in the bug report in cases where WebKit has
not painted anything into the associated layers.

This patch addresses this issue by setting up our own graphics layer in
which to paint PDF selections. To do so, we adopt the
`-[PDFSelection enumerateRectsAndTransformsForPage:usingBlock:]` SPI,
which gives us the selection geometry information necessary to fill the
current path of our graphics context.

* Source/WTF/wtf/PlatformEnableCocoa.h:

Add a compile time flag to guard the new "paint selections into our own
layer" behavior behind platform availability of requisite PDFSelection
SPI.

* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/graphics/ColorBlending.h:

Export the blendSourceOver() method, which we use to blend the selection
background color with opaque white, thus creating the final (opaque)
selection appearance.

We also annotate the blendSourceOver declaration with parameter names
for clarity, and leave a cautionary note above the blendWithWhite()
declaration.

* Source/WebCore/rendering/RenderObject.h:
* Source/WebCore/rendering/RenderTheme.h:
* Source/WebKit/Platform/spi/Cocoa/PDFKitSPI.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:

Introduce a new GraphicsLayer owned by the plugin, exclusively for
painting PDF selections. Note that this is guarded behind enablement of
the plugin painting its own selections, which itself depends on platform
availability for the requisite PDFSelection SPI.

* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::setNeedsRepaintInDocumentRect):

Make sure to ask the selection layer to be repainted if a selection is
the repaint reason.

(WebKit::UnifiedPDFPlugin::ensureLayers):
(WebKit::UnifiedPDFPlugin::updateLayerHierarchy):
(WebKit::UnifiedPDFPlugin::updateLayerPositions):
(WebKit::UnifiedPDFPlugin::didChangeSettings):
(WebKit::UnifiedPDFPlugin::layerNeedsPlatformContext const):

Make sure we return `false` under the new behavior where the plugin can
paint selections into a layer it owns. This allows us to enable
accelerated drawing for the selection layer, since it will not be backed
by the WP.

(WebKit::UnifiedPDFPlugin::didChangeIsInWindow):

Propagate isInWindow update to the selection layer such that it paints
correctly as a TiledBacking layer.

(WebKit::UnifiedPDFPlugin::paintContents):
(WebKit::UnifiedPDFPlugin::paintPDFContent):

To not vend selection painting duty to PDFKit, while also maintaining
correct selection painting behavior on platforms that do not have the
new PDFSelection SPI, make sure that haveSelection is invariably false
when we want to paint selections into our own layer.

(WebKit::UnifiedPDFPlugin::paintPDFSelection):

Avoid the problematic "multiply" compositing mode, instead painting
opaquely into the context and compositing via the selection layer, which
is configured with a "multiply" blend mode.

(WebKit::UnifiedPDFPlugin::canPaintSelectionIntoOwnedLayer const):

Returns true iff the requisite PDFSelection interface is available at
runtime.

(WebKit::UnifiedPDFPlugin::determineCurrentlySnappedPage):

Canonical link: https://commits.webkit.org/278263@main
  • Loading branch information
aprotyas committed May 2, 2024
1 parent a99db22 commit 0f33d68
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 12 deletions.
4 changes: 4 additions & 0 deletions Source/WTF/wtf/PlatformEnableCocoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,10 @@
#define ENABLE_UNIFIED_PDF_DATA_DETECTION 1
#endif

#if !defined(ENABLE_UNIFIED_PDF_SELECTION_LAYER) && ENABLE(UNIFIED_PDF) && HAVE(PDFSELECTION_ENUMERATE_RECTS_AND_TRANSFORMS)
#define ENABLE_UNIFIED_PDF_SELECTION_LAYER 1
#endif

#if !defined(ENABLE_PERIODIC_MEMORY_MONITOR) && PLATFORM(MAC)
#define ENABLE_PERIODIC_MEMORY_MONITOR 1
#endif
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/WebCore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,7 @@
334408C433F8CA4C10A328F4 /* SVGVisitedRendererTracking.h in Headers */ = {isa = PBXBuildFile; fileRef = 3344077412D9BC4A00A328F4 /* SVGVisitedRendererTracking.h */; };
33503C9A10179A74003B47E1 /* NotificationClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 33503C9910179A74003B47E1 /* NotificationClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
33503CA410179AD7003B47E1 /* JSNotification.h in Headers */ = {isa = PBXBuildFile; fileRef = 33503CA010179AD7003B47E1 /* JSNotification.h */; settings = {ATTRIBUTES = (Private, ); }; };
33B1AB242BE309F00006EFC7 /* ColorBlending.h in Headers */ = {isa = PBXBuildFile; fileRef = 7C1B4A6724A997660033727F /* ColorBlending.h */; settings = {ATTRIBUTES = (Private, ); }; };
33D0212D131DB37B004091A8 /* CookieStorage.h in Headers */ = {isa = PBXBuildFile; fileRef = E13F01EA1270E10D00DFBA71 /* CookieStorage.h */; settings = {ATTRIBUTES = (Private, ); }; };
357B581D27F7909A00B93E4E /* CSSBorderImageWidthValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 357B581C27F7909A00B93E4E /* CSSBorderImageWidthValue.h */; };
35C74FDB228A1EF6000C21A0 /* CSSGridIntegerRepeatValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 35C74FD7228A19A6000C21A0 /* CSSGridIntegerRepeatValue.h */; settings = {ATTRIBUTES = (Private, ); }; };
Expand Down Expand Up @@ -38398,6 +38399,7 @@
CD814C332996C90E005A780A /* CollectionTraversalInlines.h in Headers */,
93C442000F813AE100C1A634 /* CollectionType.h in Headers */,
B27535670B053814002CE64F /* Color.h in Headers */,
33B1AB242BE309F00006EFC7 /* ColorBlending.h in Headers */,
C330A22313EC196B0000B45B /* ColorChooser.h in Headers */,
C37CDEBD149EF2030042090D /* ColorChooserClient.h in Headers */,
F48D2AA52159740D00C6752B /* ColorCocoa.h in Headers */,
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/platform/graphics/ColorBlending.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ class Color;
// NOTE: These functions do a lossy conversion to 8-bit sRGBA before blending.

// This is an implementation of Porter-Duff's "source-over" equation.
Color blendSourceOver(const Color&, const Color&);
WEBCORE_EXPORT Color blendSourceOver(const Color& backdrop, const Color& source);

// Bespoke "whitening" algorithm used by RenderTheme::transformSelectionBackgroundColor.
// Note: This is a no-op if the color to blend with isn't opaque, which is likely not what you were expecting.
Color blendWithWhite(const Color&);

Color blend(const Color& from, const Color& to, const BlendingContext&);
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/RenderObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ class RenderObject : public CachedImageClient, public CanMakeCheckedPtr<RenderOb
void setLayoutBox(Layout::Box&);
void clearLayoutBox();

RenderTheme& theme() const;
WEBCORE_EXPORT RenderTheme& theme() const;

virtual ASCIILiteral renderName() const = 0;

Expand Down Expand Up @@ -392,7 +392,7 @@ class RenderObject : public CachedImageClient, public CanMakeCheckedPtr<RenderOb
RenderFragmentedFlow* enclosingFragmentedFlow() const;

WEBCORE_EXPORT bool useDarkAppearance() const;
OptionSet<StyleColorOptions> styleColorOptions() const;
WEBCORE_EXPORT OptionSet<StyleColorOptions> styleColorOptions() const;

#if ASSERT_ENABLED
void setHasAXObject(bool flag) { m_hasAXObject = flag; }
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/RenderTheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ class RenderTheme {
virtual bool searchFieldShouldAppearAsTextField(const RenderStyle&) const { return false; }

// Text selection colors.
Color activeSelectionBackgroundColor(OptionSet<StyleColorOptions>) const;
Color inactiveSelectionBackgroundColor(OptionSet<StyleColorOptions>) const;
WEBCORE_EXPORT Color activeSelectionBackgroundColor(OptionSet<StyleColorOptions>) const;
WEBCORE_EXPORT Color inactiveSelectionBackgroundColor(OptionSet<StyleColorOptions>) const;
virtual Color transformSelectionBackgroundColor(const Color&, OptionSet<StyleColorOptions>) const;
Color activeSelectionForegroundColor(OptionSet<StyleColorOptions>) const;
Color inactiveSelectionForegroundColor(OptionSet<StyleColorOptions>) const;
Expand Down
8 changes: 8 additions & 0 deletions Source/WebKit/Platform/spi/Cocoa/PDFKitSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ typedef NS_ENUM(NSUInteger, PDFSelectionGranularity);

#endif

#if HAVE(PDFSELECTION_ENUMERATE_RECTS_AND_TRANSFORMS)

@interface PDFSelection (Staging_125426369)
- (void)enumerateRectsAndTransformsForPage:(PDFPage *)page usingBlock:(void (^)(CGRect rect, CGAffineTransform transform))block;
@end

#endif

#endif // ENABLE(UNIFIED_PDF)

// FIXME: Move this declaration inside the !USE(APPLE_INTERNAL_SDK) block once rdar://problem/118903435 is in builds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
void notifyFlushRequired(const WebCore::GraphicsLayer*) override;
void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, const WebCore::FloatRect&, OptionSet<WebCore::GraphicsLayerPaintBehavior>) override;
float pageScaleFactor() const override;
bool layerNeedsPlatformContext(const WebCore::GraphicsLayer*) const override { return true; }
bool layerNeedsPlatformContext(const WebCore::GraphicsLayer*) const override;
void tiledBackingUsageChanged(const WebCore::GraphicsLayer*, bool /*usingTiledBacking*/) override;
std::optional<float> customContentsScale(const WebCore::GraphicsLayer*) const override;

Expand All @@ -415,6 +415,10 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
enum class PaintingBehavior : bool { All, PageContentsOnly };
enum class AllowsAsyncRendering : bool { No, Yes };
void paintPDFContent(WebCore::GraphicsContext&, const WebCore::FloatRect& clipRect, PaintingBehavior = PaintingBehavior::All, AllowsAsyncRendering = AllowsAsyncRendering::No);
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
void paintPDFSelection(WebCore::GraphicsContext&, const WebCore::FloatRect& clipRect);
#endif
bool canPaintSelectionIntoOwnedLayer() const;

void ensureLayers();
void updatePageBackgroundLayers();
Expand Down Expand Up @@ -543,6 +547,9 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
RefPtr<WebCore::GraphicsLayer> m_scrolledContentsLayer;
RefPtr<WebCore::GraphicsLayer> m_pageBackgroundsContainerLayer;
RefPtr<WebCore::GraphicsLayer> m_contentsLayer;
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
RefPtr<WebCore::GraphicsLayer> m_selectionLayer;
#endif

RefPtr<WebCore::GraphicsLayer> m_overflowControlsContainer;
RefPtr<WebCore::GraphicsLayer> m_layerForHorizontalScrollbar;
Expand Down
128 changes: 122 additions & 6 deletions Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include <WebCore/BitmapImage.h>
#include <WebCore/Chrome.h>
#include <WebCore/ChromeClient.h>
#include <WebCore/ColorBlending.h>
#include <WebCore/ColorCocoa.h>
#include <WebCore/DataDetectorElementInfo.h>
#include <WebCore/DictionaryLookup.h>
Expand All @@ -68,6 +69,7 @@
#include <WebCore/GraphicsContext.h>
#include <WebCore/GraphicsLayer.h>
#include <WebCore/GraphicsLayerClient.h>
#include <WebCore/GraphicsTypes.h>
#include <WebCore/HTMLNames.h>
#include <WebCore/HTMLPlugInElement.h>
#include <WebCore/ImageBuffer.h>
Expand Down Expand Up @@ -385,6 +387,14 @@ static String mutationObserverNotificationString()
asyncRenderer->pdfContentChangedInRect(m_scaleFactor, contentsRect);
}

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
if (repaintRequirements.contains(RepaintRequirement::Selection) && canPaintSelectionIntoOwnedLayer()) {
RefPtr { m_selectionLayer }->setNeedsDisplayInRect(contentsRect);
if (repaintRequirements.hasExactlyOneBitSet())
return;
}
#endif

RefPtr { m_contentsLayer }->setNeedsDisplayInRect(contentsRect);
}

Expand Down Expand Up @@ -450,6 +460,17 @@ static String mutationObserverNotificationString()
m_overflowControlsContainer->setAnchorPoint({ });
m_rootLayer->addChild(*m_overflowControlsContainer);
}

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
if (!m_selectionLayer) {
m_selectionLayer = createGraphicsLayer("PDF selections"_s, GraphicsLayer::Type::TiledBacking);
m_selectionLayer->setAnchorPoint({ });
m_selectionLayer->setDrawsContent(true);
m_selectionLayer->setAcceleratesDrawing(true);
m_selectionLayer->setBlendMode(BlendMode::Multiply);
m_scrolledContentsLayer->addChild(*m_selectionLayer);
}
#endif
}

void UnifiedPDFPlugin::updatePageBackgroundLayers()
Expand Down Expand Up @@ -667,6 +688,11 @@ static String mutationObserverNotificationString()
m_contentsLayer->setSize(documentSize());
m_contentsLayer->setNeedsDisplay();

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
m_selectionLayer->setSize(documentSize());
m_selectionLayer->setNeedsDisplay();
#endif

updatePageBackgroundLayers();
updateSnapOffsets();

Expand All @@ -682,6 +708,9 @@ static String mutationObserverNotificationString()
transform.translate(padding.width(), padding.height());

m_contentsLayer->setTransform(transform);
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
m_selectionLayer->setTransform(transform);
#endif
m_pageBackgroundsContainerLayer->setTransform(transform);
}

Expand Down Expand Up @@ -709,6 +738,9 @@ static String mutationObserverNotificationString()
propagateSettingsToLayer(*m_scrolledContentsLayer);
propagateSettingsToLayer(*m_pageBackgroundsContainerLayer);
propagateSettingsToLayer(*m_contentsLayer);
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
propagateSettingsToLayer(*m_selectionLayer);
#endif

for (auto& pageLayer : m_pageBackgroundsContainerLayer->children()) {
propagateSettingsToLayer(pageLayer);
Expand Down Expand Up @@ -742,6 +774,13 @@ static String mutationObserverNotificationString()
return { };
}

bool UnifiedPDFPlugin::layerNeedsPlatformContext(const WebCore::GraphicsLayer*) const
{
// We need a platform context if the plugin can not paint selections into its own layer,
// since we would then have to vend a platform context that PDFKit can paint into.
return !canPaintSelectionIntoOwnedLayer();
}

void UnifiedPDFPlugin::tiledBackingUsageChanged(const GraphicsLayer* layer, bool usingTiledBacking)
{
RefPtr page = this->page();
Expand All @@ -758,13 +797,21 @@ static String mutationObserverNotificationString()
if (!page || !m_contentsLayer)
return;

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
if (!m_selectionLayer)
return;
#endif

bool isInWindow = page->isInWindow();
m_contentsLayer->setIsInWindow(isInWindow);
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
m_selectionLayer->setIsInWindow(isInWindow);
#endif

for (auto& pageLayer : m_pageBackgroundsContainerLayer->children()) {
if (pageLayer->children().size()) {
Ref pageContensLayer = pageLayer->children()[0];
pageContensLayer->setIsInWindow(isInWindow);
Ref pageContentsLayer = pageLayer->children()[0];
pageContentsLayer->setIsInWindow(isInWindow);
}
}
}
Expand Down Expand Up @@ -833,6 +880,11 @@ static String mutationObserverNotificationString()
return;
}

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
if (layer == m_selectionLayer.get())
return paintPDFSelection(context, clipRect);
#endif

if (auto backgroundLayerPageIndex = pageIndexForPageBackgroundLayer(layer)) {
paintBackgroundLayerForPage(layer, context, clipRect, *backgroundLayerPageIndex);
return;
Expand Down Expand Up @@ -883,16 +935,14 @@ static String mutationObserverNotificationString()
if (m_size.isEmpty() || documentSize().isEmpty())
return;

bool shouldPaintSelection = behavior == PaintingBehavior::All;

auto stateSaver = GraphicsContextStateSaver(context);

auto showDebugIndicators = shouldShowDebugIndicators();

bool haveSelection = false;
bool isVisibleAndActive = false;
if (m_currentSelection && shouldPaintSelection) {
// FIXME: Also test is m_currentSelection is not empty.
bool shouldPaintSelection = behavior == PaintingBehavior::All && !canPaintSelectionIntoOwnedLayer();
if (m_currentSelection && ![m_currentSelection isEmpty] && shouldPaintSelection) {
haveSelection = true;
if (RefPtr page = this->page())
isVisibleAndActive = page->isVisibleAndActive();
Expand Down Expand Up @@ -968,6 +1018,69 @@ static String mutationObserverNotificationString()
}
}

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
void UnifiedPDFPlugin::paintPDFSelection(GraphicsContext& context, const FloatRect& clipRect)
{
if (!m_currentSelection || [m_currentSelection isEmpty] || !canPaintSelectionIntoOwnedLayer())
return;

bool isVisibleAndActive = false;
if (RefPtr page = this->page())
isVisibleAndActive = page->isVisibleAndActive();

auto selectionColor = [renderer = m_element->renderer(), isVisibleAndActive] {
auto& renderTheme = renderer->theme();
auto styleColorOptions = renderer->styleColorOptions();
auto selectionColor = isVisibleAndActive ? renderTheme.activeSelectionBackgroundColor(styleColorOptions) : renderTheme.inactiveSelectionBackgroundColor(styleColorOptions);
return blendSourceOver(Color::white, selectionColor);
}();

auto pageCoverage = pageCoverageForRect(clipRect);
auto documentScale = pageCoverage.pdfDocumentScale;
for (auto& pageInfo : pageCoverage.pages) {
auto page = m_documentLayout.pageAtIndex(pageInfo.pageIndex);
if (!page || !shouldDisplayPage(pageInfo.pageIndex))
continue;

auto pageDestinationRect = pageInfo.pageBounds;

GraphicsContextStateSaver pageStateSaver { context };
context.scale(documentScale);
context.clip(pageDestinationRect);
// Translate the context to the bottom of pageBounds and flip, so that PDFKit operates
// from this page's drawing origin.
context.translate(pageDestinationRect.minXMaxYCorner());
context.scale({ 1, -1 });

auto pageGeometry = m_documentLayout.geometryForPage(page);
auto transformForBox = m_documentLayout.toPageTransform(*pageGeometry).inverse().value_or(AffineTransform { });
context.concatCTM(transformForBox);

if ([m_currentSelection respondsToSelector:@selector(enumerateRectsAndTransformsForPage:usingBlock:)]) {
[protectedCurrentSelection() enumerateRectsAndTransformsForPage:page.get() usingBlock:[&context, &selectionColor](CGRect cgRect, CGAffineTransform cgTransform) mutable {
// FIXME: Perf optimization -- consider coalescing rects by transform.
GraphicsContextStateSaver individualRectTransformPairStateSaver { context, /* saveAndRestore */ false };

if (AffineTransform transform { cgTransform }; !transform.isIdentity()) {
individualRectTransformPairStateSaver.save();
context.concatCTM(transform);
}

context.fillRect({ cgRect }, selectionColor);
}];
}
}
}
#endif

bool UnifiedPDFPlugin::canPaintSelectionIntoOwnedLayer() const
{
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
return [m_currentSelection respondsToSelector:@selector(enumerateRectsAndTransformsForPage:usingBlock:)];
#endif
return false;
}

static const WebCore::Color textAnnotationHoverColor()
{
static constexpr auto textAnnotationHoverAlpha = 0.12;
Expand Down Expand Up @@ -1780,6 +1893,9 @@ static String mutationObserverNotificationString()
m_currentlySnappedPage = newSnappedPage;
updatePageBackgroundLayers();
m_contentsLayer->setNeedsDisplay();
#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
m_selectionLayer->setNeedsDisplay();
#endif
}
}

Expand Down

0 comments on commit 0f33d68

Please sign in to comment.