Skip to content

Commit

Permalink
[UnifiedPDF] PDFDocumentLayout::nearestPageIndexForDocumentPoint some…
Browse files Browse the repository at this point in the history
…times returns the incorrect page index

https://bugs.webkit.org/show_bug.cgi?id=271777
rdar://125502090

Reviewed by Abrar Rahman Protyasha.

The current heuristic worked well for the purposes of next/previous page
context menu navigation, but it likely will not be sufficient to adopt
anywhere else. In order to make this much more precise in two up mode,
we need to rework the heuristic. There are two main changes:

1.) Now iterate over the pairs of pages rather than individual ones.
This makes the rest of the logic much more intuitive since in two up
mode we are almost always working with pairs of pages. There are cases
where we may only have one page, but this is handled naturally by the
fact that the second page in PairedPageLayoutBounds is optional.

2.) Instead of picking the page with the closer x coordinate after
determining the pair of pages to consider, we should instead compare
the distance from the point in document space to various points on the
pages. In particular, we should compare the distance to the corners of
the page and to the side of the page it is next to (if possible). The
smallest distance between all of these points should represent the page
closest to the point in question.

One other change I made is related to the logic when the document point
is below the PDF. In this case we should perform the same type of logic
with the last pages instead of blindly returning the last page index
like we were doing before.

* Source/WebCore/platform/graphics/GeometryUtilities.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDocumentLayout.mm:
(WebKit::PDFDocumentLayout::nearestPageIndexForDocumentPoint const):

Canonical link: https://commits.webkit.org/276852@main
  • Loading branch information
sammygill committed Mar 30, 2024
1 parent c9cb06e commit 59d1217
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 17 deletions.
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/GeometryUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace WebCore {
class FloatQuad;

float euclidianDistance(const FloatSize&);
float euclidianDistance(const FloatPoint&, const FloatPoint&);
WEBCORE_EXPORT float euclidianDistance(const FloatPoint&, const FloatPoint&);

// Find point where lines through the two pairs of points intersect. Returns false if the lines don't intersect.
WEBCORE_EXPORT bool findIntersection(const FloatPoint& p1, const FloatPoint& p2, const FloatPoint& d1, const FloatPoint& d2, FloatPoint& intersection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#import "Logging.h"
#import <WebCore/AffineTransform.h>
#import <WebCore/GeometryUtilities.h>
#import <wtf/text/TextStream.h>

#import "PDFKitSoftLink.h"
Expand Down Expand Up @@ -77,28 +78,78 @@
auto pageCount = this->pageCount();
switch (displayMode()) {
case PDFDocumentLayout::DisplayMode::TwoUpDiscrete:
case PDFDocumentLayout::DisplayMode::TwoUpContinuous:
for (PDFDocumentLayout::PageIndex index = 0; index < pageCount; ++index) {
if (index == pageCount - 1)
return index;
case PDFDocumentLayout::DisplayMode::TwoUpContinuous: {

using PairedPagesLayoutBounds = std::pair<FloatRect, std::optional<FloatRect>>;

auto layoutBoundsForPairedPages = [this](PageIndex pageIndex) -> PairedPagesLayoutBounds {
if (isRightPageIndex(pageIndex))
return { layoutBoundsForPageAtIndex(pageIndex - 1), layoutBoundsForPageAtIndex(pageIndex) };
if (isLastPageIndex(pageIndex))
return { layoutBoundsForPageAtIndex(pageIndex), { } };
return { layoutBoundsForPageAtIndex(pageIndex), layoutBoundsForPageAtIndex(pageIndex + 1) };
};

auto minimumDistanceToPage = [documentSpacePoint](const FloatRect& pageLayoutBounds) {
if (pageLayoutBounds.contains(documentSpacePoint))
return 0.0f;

auto pointsToCompare = Vector<FloatPoint> { pageLayoutBounds.minXMinYCorner(), pageLayoutBounds.minXMaxYCorner(), pageLayoutBounds.maxXMinYCorner(), pageLayoutBounds.maxXMaxYCorner() };

bool isAbovePage = documentSpacePoint.y() < pageLayoutBounds.y();
bool isBelowPage = documentSpacePoint.y() > pageLayoutBounds.maxY();
bool isLeftOfPage = documentSpacePoint.x() < pageLayoutBounds.x();
bool isRightOfPage = documentSpacePoint.x() > pageLayoutBounds.maxX();

bool isWithinPageWidth = isInRange(documentSpacePoint.x(), pageLayoutBounds.x(), pageLayoutBounds.maxX());
bool isWithinPageHeight = isInRange(documentSpacePoint.y(), pageLayoutBounds.y(), pageLayoutBounds.maxY());

if (isAbovePage && isWithinPageWidth)
pointsToCompare.append({ documentSpacePoint.x(), pageLayoutBounds.y() });
else if (isRightOfPage && isWithinPageHeight)
pointsToCompare.append({ pageLayoutBounds.x(), documentSpacePoint.y() });
else if (isBelowPage && isWithinPageWidth)
pointsToCompare.append({ documentSpacePoint.x(), pageLayoutBounds.maxY() });
else if (isLeftOfPage && isWithinPageHeight)
pointsToCompare.append({ pageLayoutBounds.x(), documentSpacePoint.y() });

auto distancesToPoints = WTF::map(pointsToCompare, [documentSpacePoint](FloatPoint point) {
return euclidianDistance(point, documentSpacePoint);
});

auto currentPageBounds = layoutBoundsForPageAtIndex(index);
if (documentSpacePoint.y() < currentPageBounds.maxY()) {
return *std::min_element(distancesToPoints.begin(), distancesToPoints.end());
};

auto pairedPageIndex = [index, this]() {
if (index % pagesPerRow())
return index - 1;
for (PageIndex index = 0; index < pageCount; index += pagesPerRow()) {
auto [leftPageLayoutBounds, rightPageLayoutBounds] = layoutBoundsForPairedPages(index);

if (documentSpacePoint.y() < leftPageLayoutBounds.maxY() || (rightPageLayoutBounds && documentSpacePoint.y() < rightPageLayoutBounds->maxY())) {

if (!rightPageLayoutBounds || leftPageLayoutBounds.contains(documentSpacePoint))
return index;

if (rightPageLayoutBounds->contains(documentSpacePoint))
return index + 1;
}();

auto pairedPageMaxX= layoutBoundsForPageAtIndex(pairedPageIndex).maxX();
auto currentPageMaxX = currentPageBounds.maxX();
if (currentPageMaxX < pairedPageMaxX)
return documentSpacePoint.x() < currentPageMaxX ? index : pairedPageIndex;
return documentSpacePoint.x() < pairedPageMaxX ? pairedPageIndex : index;
if (minimumDistanceToPage(leftPageLayoutBounds) < minimumDistanceToPage(rightPageLayoutBounds.value()))
return index;

return index + 1;
}
}
break;

auto lastPageIndex = pageCount - 1;
if (isLeftPageIndex(lastPageIndex))
return lastPageIndex;

auto [leftPageLayoutBounds, rightPageLayoutBounds] = layoutBoundsForPairedPages(lastPageIndex);
ASSERT(rightPageLayoutBounds);

if (minimumDistanceToPage(leftPageLayoutBounds) < minimumDistanceToPage(rightPageLayoutBounds.value_or(FloatRect { })))
return lastPageIndex - 1;

return lastPageIndex;
}
case PDFDocumentLayout::DisplayMode::SinglePageDiscrete:
case PDFDocumentLayout::DisplayMode::SinglePageContinuous: {
for (PDFDocumentLayout::PageIndex index = 0; index < pageCount; ++index) {
Expand Down

0 comments on commit 59d1217

Please sign in to comment.