Skip to content

Commit

Permalink
[UnifiedPDF] Data detector page overlay tiles should not be repainted…
Browse files Browse the repository at this point in the history
… when there is no active highlight

https://bugs.webkit.org/show_bug.cgi?id=271074
rdar://124707729

Reviewed by Simon Fraser.

Currently, we unconditionally call PageOverlay::setNeedsDisplay()
whenever we scroll the PDF plugin. This is incredibly inefficient
because we're repainting all the tiles even when there is no need to do
so.

This patch teaches teh data detector overlay controller to better reason
about when a repaint is necessary, and only calls setNeedsDisplay() when
those conditions are met. Namely, we no longer repaint if there is no
active highlight or if the active highlight instance has not changed.

In a future patch, we will further optimize this by clipping the repaint
to only the bounds of the active (and just-deactivated) highlight.

* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDataDetectorOverlayController.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDataDetectorOverlayController.mm:
(WebKit::PDFDataDetectorOverlayController::handleMouseEvent):
(WebKit::PDFDataDetectorOverlayController::didInvalidateHighlightOverlayRects):

Canonical link: https://commits.webkit.org/276200@main
  • Loading branch information
aprotyas committed Mar 15, 2024
1 parent fd02524 commit 3aad301
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class PDFDataDetectorOverlayController final : private PageOverlay::Client, WebC
RefPtr<PageOverlay> protectedOverlay() const { return m_overlay; }

enum class ShouldUpdatePlatformHighlightData : bool { No, Yes };
void didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData = ShouldUpdatePlatformHighlightData::Yes);
enum class ActiveHighlightChanged : bool { No, Yes };
void didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData = ShouldUpdatePlatformHighlightData::Yes, ActiveHighlightChanged = ActiveHighlightChanged::No);

private:
// PageOverlay::Client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@
m_activeDataDetectorHighlight->fadeIn();
}

overlay->setNeedsDisplay();
didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData::No);
didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData::No, ActiveHighlightChanged::Yes);
}

if (event.type() == WebEventType::MouseDown && mouseIsOverActiveHighlightButton)
Expand Down Expand Up @@ -234,14 +233,17 @@
});
}

void PDFDataDetectorOverlayController::didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData shouldUpdatePlatformHighlightData)
void PDFDataDetectorOverlayController::didInvalidateHighlightOverlayRects(ShouldUpdatePlatformHighlightData shouldUpdatePlatformHighlightData, ActiveHighlightChanged activeHighlightChanged)
{
if (shouldUpdatePlatformHighlightData == ShouldUpdatePlatformHighlightData::Yes) {
WTF::forEach(m_pdfDataDetectorItemsWithHighlightsMap.keys(), [&](auto pageIndex) {
updatePlatformHighlightData(pageIndex);
});
}

if (activeHighlightChanged == ActiveHighlightChanged::No && !m_activeDataDetectorHighlight)
return;

RefPtr overlay = protectedOverlay();

// FIXME: Should specify the DDHighlight bounds as the dirty rect instead of repainting the world.
Expand Down

0 comments on commit 3aad301

Please sign in to comment.