Skip to content

Commit

Permalink
UnifiedPDF: Page previews disappear when zoomed enough
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=284458
rdar://141280757

Reviewed by Simon Fraser.

Page overview layer would turn to tiled backing layer when zoomed enough.
Changing to tiled backing would change GraphicsLayer primary layer id.
The presentation controller would disambiguate what to paint as contents
based on which primary layer the updated GraphicsLayer has. Since
the presentation controller would not know the new id, the layer would
stay empty.

Fix by:
Force the background layer to not allow tiling.

To avoid similar bugs in the future, disambiguate the contents based on
the GraphicsLayer, not based on the primary layer id of the
GraphicsLayer. The primary layer is a implementation detail of the
GraphicsLayer, and its behavior cannot be anticipated by the client,
PDFPresentationController. AsyncPDFRenderer has similar problem, but
this cannot be solved as we cannot navigate from tile grid id to
graphics layer.

* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::changeLayerTypeTo):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/AsyncPDFRenderer.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/AsyncPDFRenderer.mm:
(WebKit::AsyncPDFRenderer::releaseMemory):
(WebKit::AsyncPDFRenderer::startTrackingLayer):
(WebKit::AsyncPDFRenderer::stopTrackingLayer):
(WebKit::AsyncPDFRenderer::layerForTileGrid const):
(WebKit::AsyncPDFRenderer::coverageRectDidChange):
(WebKit::AsyncPDFRenderer::renderInfoForTile const):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.mm:
(WebKit::PDFDiscretePresentationController::buildRows):
(WebKit::PDFDiscretePresentationController::customContentsScale const):
(WebKit::PDFDiscretePresentationController::layerNeedsPlatformContext const):
(WebKit::PDFDiscretePresentationController::rowDataForLayer const const):
(WebKit::PDFDiscretePresentationController::rowForLayer const):
(WebKit::PDFDiscretePresentationController::paintContents):
(WebKit::PDFDiscretePresentationController::rowDataForLayerID const const): Deleted.
(WebKit::PDFDiscretePresentationController::rowForLayerID const): Deleted.
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.h:
(WebKit::PDFPresentationController::rowForLayer const):
(WebKit::PDFPresentationController::rowForLayerID const): Deleted.
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.mm:
(WebKit::PDFPresentationController::makePageContainerLayer):

Canonical link: https://commits.webkit.org/287788@main
  • Loading branch information
kkinnunen-apple committed Dec 13, 2024
1 parent c1b41e0 commit 2b42362
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class AsyncPDFRenderer final : public WebCore::TiledBackingClient,
private:
AsyncPDFRenderer(PDFPresentationController&);

WebCore::GraphicsLayer* layerForTileGrid(WebCore::TileGridIdentifier) const;
RefPtr<WebCore::GraphicsLayer> layerForTileGrid(WebCore::TileGridIdentifier) const;

TileRenderInfo renderInfoForFullTile(const WebCore::TiledBacking&, const TileForGrid& tileInfo, const WebCore::FloatRect& tileRect) const;
TileRenderInfo renderInfoForTile(const WebCore::TiledBacking&, const TileForGrid& tileInfo, const WebCore::FloatRect& tileRect, const WebCore::FloatRect& renderRect, RefPtr<WebCore::NativeImage>&& background) const;
Expand Down
47 changes: 21 additions & 26 deletions Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/AsyncPDFRenderer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -84,39 +84,38 @@
if (!presentationController)
return;

for (auto& [layerID, layer] : m_layerIDtoLayerMap) {
for (Ref layer : m_layerIDtoLayerMap.values()) {
// Ideally we'd be able to make the ImageBuffer memory volatile which would eliminate the need for this callback: webkit.org/b/274878
if (auto* tiledBacking = layer->tiledBacking())
removePagePreviewsOutsideCoverageRect(tiledBacking->coverageRect(), presentationController->rowForLayerID(layerID));
removePagePreviewsOutsideCoverageRect(tiledBacking->coverageRect(), presentationController->rowForLayer(layer.ptr()));
}

LOG_WITH_STREAM(PDFAsyncRendering, stream << "AsyncPDFRenderer::releaseMemory - reduced page preview count from " << oldPagePreviewCount << " to " << m_pagePreviews.size());
}

void AsyncPDFRenderer::startTrackingLayer(GraphicsLayer& layer)
{
Ref layerRef = layer;
if (auto* tiledBacking = layerRef->tiledBacking()) {
tiledBacking->setClient(this);
m_tileGridToLayerIDMap.set(tiledBacking->primaryGridIdentifier(), *layer.primaryLayerID());
}

m_layerIDtoLayerMap.set(*layer.primaryLayerID(), WTFMove(layerRef));
auto* tiledBacking = layer.tiledBacking();
if (!tiledBacking)
return;
tiledBacking->setClient(this);
m_tileGridToLayerIDMap.set(tiledBacking->primaryGridIdentifier(), *layer.primaryLayerID());
m_layerIDtoLayerMap.set(*layer.primaryLayerID(), layer);
}

void AsyncPDFRenderer::stopTrackingLayer(GraphicsLayer& layer)
{
if (auto* tiledBacking = layer.tiledBacking()) {
auto gridIdentifier = tiledBacking->primaryGridIdentifier();
m_tileGridToLayerIDMap.remove(gridIdentifier);
m_gridRevalidationState.remove(gridIdentifier);
tiledBacking->setClient(nullptr);
}

auto* tiledBacking = layer.tiledBacking();
if (!tiledBacking)
return;
auto gridIdentifier = tiledBacking->primaryGridIdentifier();
m_tileGridToLayerIDMap.remove(gridIdentifier);
m_gridRevalidationState.remove(gridIdentifier);
tiledBacking->setClient(nullptr);
m_layerIDtoLayerMap.remove(*layer.primaryLayerID());
}

GraphicsLayer* AsyncPDFRenderer::layerForTileGrid(TileGridIdentifier identifier) const
RefPtr<GraphicsLayer> AsyncPDFRenderer::layerForTileGrid(TileGridIdentifier identifier) const
{
auto layerID = m_tileGridToLayerIDMap.getOptional(identifier);
if (!layerID)
Expand Down Expand Up @@ -286,12 +285,9 @@
return;

std::optional<PDFLayoutRow> layoutRow;
RefPtr<GraphicsLayer> layer;
auto layerID = m_tileGridToLayerIDMap.getOptional(tiledBacking.primaryGridIdentifier());
if (layerID) {
layoutRow = presentationController->rowForLayerID(*layerID);
layer = m_layerIDtoLayerMap.get(*layerID);
}
RefPtr<GraphicsLayer> layer = layerForTileGrid(tiledBacking.primaryGridIdentifier());
if (layer)
layoutRow = presentationController->rowForLayer(layer.get());

auto pageCoverage = presentationController->pageCoverageForContentsRect(coverageRect, layoutRow);

Expand Down Expand Up @@ -506,9 +502,8 @@
auto paintingClipRect = convertTileRectToPaintingCoords(tileRect, tilingScaleFactor);

std::optional<PDFLayoutRow> layoutRow;
auto layerID = m_tileGridToLayerIDMap.getOptional(tileInfo.gridIdentifier);
if (layerID)
layoutRow = presentationController->rowForLayerID(*layerID);
if (RefPtr layer = layerForTileGrid(tileInfo.gridIdentifier))
layoutRow = presentationController->rowForLayer(layer.get());

auto pageCoverage = presentationController->pageCoverageAndScalesForContentsRect(paintingClipRect, layoutRow, tilingScaleFactor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class PDFDiscretePresentationController final : public PDFPresentationController
WebCore::GraphicsLayerClient& graphicsLayerClient() override { return *this; }

std::optional<PDFLayoutRow> visibleRow() const override;
std::optional<PDFLayoutRow> rowForLayerID(WebCore::PlatformLayerIdentifier) const override;
std::optional<PDFLayoutRow> rowForLayer(const WebCore::GraphicsLayer*) const override;

WebCore::FloatSize contentsOffsetForPage(PDFDocumentLayout::PageIndex) const;

Expand Down Expand Up @@ -216,7 +216,7 @@ class PDFDiscretePresentationController final : public PDFPresentationController
#endif
};

const RowData* rowDataForLayerID(WebCore::PlatformLayerIdentifier) const;
const RowData* rowDataForLayer(const WebCore::GraphicsLayer*) const;
WebCore::FloatPoint positionForRowContainerLayer(const PDFLayoutRow&) const;
WebCore::FloatSize rowContainerSize(const PDFLayoutRow&) const;

Expand All @@ -225,7 +225,7 @@ class PDFDiscretePresentationController final : public PDFPresentationController
RefPtr<WebCore::GraphicsLayer> m_rowsContainerLayer;
Vector<RowData> m_rows;

HashMap<WebCore::PlatformLayerIdentifier, unsigned> m_layerIDToRowIndexMap;
HashMap<const WebCore::GraphicsLayer*, unsigned> m_layerToRowIndexMap;
std::optional<PDFDocumentLayout::DisplayMode> m_displayModeAtLastLayerSetup;

unsigned m_visibleRowIndex { 0 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci

row.leftPageContainerLayer = makePageContainerLayer(leftPageIndex);
RefPtr pageBackgroundLayer = pageBackgroundLayerForPageContainerLayer(*row.protectedLeftPageContainerLayer());
m_layerIDToRowIndexMap.add(*pageBackgroundLayer->primaryLayerID(), rowIndex);
m_layerToRowIndexMap.add(pageBackgroundLayer.get(), rowIndex);

if (row.pages.numPages() == 1) {
ASSERT(!row.rightPageContainerLayer);
Expand All @@ -1079,7 +1079,7 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci
auto rightPageIndex = layoutRow.pages[1];
row.rightPageContainerLayer = makePageContainerLayer(rightPageIndex);
RefPtr rightPageBackgroundLayer = pageBackgroundLayerForPageContainerLayer(*row.protectedRightPageContainerLayer());
m_layerIDToRowIndexMap.add(*rightPageBackgroundLayer->primaryLayerID(), rowIndex);
m_layerToRowIndexMap.add(rightPageBackgroundLayer.get(), rowIndex);
};

auto parentRowLayers = [](RowData& row) {
Expand Down Expand Up @@ -1112,15 +1112,15 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci
// This is the call that enables async rendering.
asyncRenderer()->startTrackingLayer(*rowContentsLayer);

m_layerIDToRowIndexMap.set(*rowContentsLayer->primaryLayerID(), rowIndex);
m_layerToRowIndexMap.set(rowContentsLayer.get(), rowIndex);

#if ENABLE(UNIFIED_PDF_SELECTION_LAYER)
RefPtr rowSelectionLayer = row.selectionLayer = createGraphicsLayer(makeString("Row selection "_s, rowIndex), GraphicsLayer::Type::TiledBacking);
rowSelectionLayer->setAnchorPoint({ });
rowSelectionLayer->setDrawsContent(true);
rowSelectionLayer->setAcceleratesDrawing(true);
rowSelectionLayer->setBlendMode(BlendMode::Multiply);
m_layerIDToRowIndexMap.set(*rowSelectionLayer->primaryLayerID(), rowIndex);
m_layerToRowIndexMap.set(rowSelectionLayer.get(), rowIndex);
#endif

parentRowLayers(row);
Expand Down Expand Up @@ -1446,7 +1446,7 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci

std::optional<float> PDFDiscretePresentationController::customContentsScale(const GraphicsLayer* layer) const
{
auto* rowData = rowDataForLayerID(*layer->primaryLayerID());
auto* rowData = rowDataForLayer(layer);
if (!rowData)
return { };

Expand All @@ -1464,7 +1464,7 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci
// 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.
// However, this constraint only applies for the contents layer. No other layer needs to be WP-backed.
auto* rowData = rowDataForLayerID(*layer->primaryLayerID());
auto* rowData = rowDataForLayer(layer);
if (!rowData)
return false;

Expand Down Expand Up @@ -1503,9 +1503,9 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci
}
}

auto PDFDiscretePresentationController::rowDataForLayerID(PlatformLayerIdentifier layerID) const -> const RowData*
auto PDFDiscretePresentationController::rowDataForLayer(const GraphicsLayer* layer) const -> const RowData*
{
auto rowIndex = m_layerIDToRowIndexMap.getOptional(layerID);
auto rowIndex = m_layerToRowIndexMap.getOptional(layer);
if (!rowIndex)
return nullptr;

Expand All @@ -1523,9 +1523,9 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci
return m_rows[m_visibleRowIndex].pages;
}

std::optional<PDFLayoutRow> PDFDiscretePresentationController::rowForLayerID(PlatformLayerIdentifier layerID) const
std::optional<PDFLayoutRow> PDFDiscretePresentationController::rowForLayer(const GraphicsLayer* layer) const
{
auto* rowData = rowDataForLayerID(layerID);
auto* rowData = rowDataForLayer(layer);
if (rowData)
return rowData->pages;

Expand All @@ -1534,7 +1534,7 @@ static float elasticDeltaForTimeDelta(float initialPosition, float initialVeloci

void PDFDiscretePresentationController::paintContents(const GraphicsLayer* layer, GraphicsContext& context, const FloatRect& clipRect, OptionSet<GraphicsLayerPaintBehavior>)
{
auto rowIndex = m_layerIDToRowIndexMap.getOptional(*layer->primaryLayerID());
auto rowIndex = m_layerToRowIndexMap.getOptional(layer);
if (!rowIndex)
return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class PDFPresentationController : public ThreadSafeRefCountedAndCanMakeThreadSaf
virtual void setNeedsRepaintInDocumentRect(OptionSet<RepaintRequirement>, const WebCore::FloatRect& rectInDocumentCoordinates, std::optional<PDFLayoutRow>) = 0;

virtual std::optional<PDFLayoutRow> visibleRow() const { return { }; }
virtual std::optional<PDFLayoutRow> rowForLayerID(WebCore::PlatformLayerIdentifier) const { return { }; }
virtual std::optional<PDFLayoutRow> rowForLayer(const WebCore::GraphicsLayer*) const { return { }; }

struct VisiblePDFPosition {
PDFDocumentLayout::PageIndex pageIndex { 0 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
pageBackgroundLayer->setDrawsContent(true);
pageBackgroundLayer->setAcceleratesDrawing(true);
pageBackgroundLayer->setShouldUpdateRootRelativeScaleFactor(false);
pageBackgroundLayer->setAllowsTiling(false);
pageBackgroundLayer->setNeedsDisplay(); // We only need to paint this layer once when page backgrounds change.

// FIXME: <https://webkit.org/b/276981> Need to add a 1px black border with alpha 0.0586.
Expand Down

0 comments on commit 2b42362

Please sign in to comment.