Skip to content

Commit

Permalink
[PDF] Clean up some scales and #ifdefs in PDFPlugin code
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=271912
rdar://125630155

Reviewed by Tim Horton.

Various minor bits of cleanup in PDFPlugin code:
1. Make all the scales be doubles
2. Make `contentScaleFactor()` internal to PDFPluginBase and subclasses,
   requiring some annotation classes to be friends. Stop exposing it via
   PluginView.
3. Change some ENABLE(LEGACY_PDFKIT_PLUGIN) to ENABLE(PDF_PLUGIN) for classes
   that are still used with UnifiedPDF

* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::showDefinitionForAttributedString):
(WebKit::PDFPlugin::scaleFactor const):
(WebKit::PDFPlugin::contentScaleFactor const):
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.mm:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.h:
(WebKit::PDFPluginBase::didEndMagnificationGesture):
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.mm:
(WebKit::PDFPluginBase::dictionaryPopupInfoForSelection):
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginChoiceAnnotation.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginChoiceAnnotation.mm:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginPasswordField.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginPasswordField.mm:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::scaleForActualSize const):
(WebKit::UnifiedPDFPlugin::scaleForFitToView const):
(WebKit::UnifiedPDFPlugin::initialScale const):
(WebKit::UnifiedPDFPlugin::scaleFactor const):
(WebKit::UnifiedPDFPlugin::contentScaleFactor const):
(WebKit::UnifiedPDFPlugin::centeringOffset const):
* Source/WebKit/WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::contentScaleFactor const): Deleted.
* Source/WebKit/WebProcess/Plugins/PluginView.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::pageZoomFactor const):
(WebKit::WebPage::setPageZoomFactor):

Canonical link: https://commits.webkit.org/276855@main
  • Loading branch information
smfr committed Mar 30, 2024
1 parent e95ad59 commit 7706664
Show file tree
Hide file tree
Showing 17 changed files with 55 additions and 59 deletions.
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ class PDFPlugin final : public PDFPluginBase {
WebCore::IntPoint convertFromRootViewToPDFView(const WebCore::IntPoint&) const;
WebCore::FloatRect convertFromPDFViewToScreen(const WebCore::FloatRect&) const;

CGFloat scaleFactor() const override;
float contentScaleFactor() const final;
double scaleFactor() const override;
double contentScaleFactor() const final;
CGSize contentSizeRespectingZoom() const;

private:
Expand Down
6 changes: 3 additions & 3 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@ static bool getEventTypeFromWebEvent(const WebEvent& event, NSEventType& eventTy

NSRect rangeRect;
rangeRect.origin = NSMakePoint(point.x, point.y);
CGFloat scaleFactor = PDFPlugin::scaleFactor();
auto scaleFactor = PDFPlugin::scaleFactor();

rangeRect.size.height = string.size.height * scaleFactor;
rangeRect.size.width = string.size.width * scaleFactor;
Expand Down Expand Up @@ -1536,12 +1536,12 @@ static NSRect rectInViewSpaceForRectInLayoutSpace(PDFLayerController* pdfLayerCo
return [m_pdfLayerController contentSizeRespectingZoom];
}

CGFloat PDFPlugin::scaleFactor() const
double PDFPlugin::scaleFactor() const
{
return [m_pdfLayerController contentScaleFactor];
}

float PDFPlugin::contentScaleFactor() const
double PDFPlugin::contentScaleFactor() const
{
return scaleFactor();
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

#pragma once

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#include <WebCore/EventListener.h>
#include <wtf/RefCounted.h>
Expand Down Expand Up @@ -109,4 +109,4 @@ class PDFPluginAnnotation : public RefCounted<PDFPluginAnnotation> {

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
9 changes: 5 additions & 4 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@
#import "config.h"
#import "PDFPluginAnnotation.h"

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#import "PDFLayerControllerSPI.h"
#import "PDFPlugin.h"
#import "PDFPluginBase.h"
#import "PDFPluginChoiceAnnotation.h"
#import "PDFPluginTextAnnotation.h"
#import <Quartz/Quartz.h>
#import <CoreGraphics/CoreGraphics.h>
#import <PDFKit/PDFKit.h>
#import <WebCore/AddEventListenerOptions.h>
#import <WebCore/CSSPrimitiveValue.h>
#import <WebCore/CSSPropertyNames.h>
Expand Down Expand Up @@ -106,7 +107,7 @@

void PDFPluginAnnotation::updateGeometry()
{
NSRect annotationRect = NSRectFromCGRect(m_plugin->pluginBoundsForAnnotation(m_annotation));
auto annotationRect = m_plugin->pluginBoundsForAnnotation(m_annotation);

StyledElement* styledElement = static_cast<StyledElement*>(element());
styledElement->setInlineStyleProperty(CSSPropertyWidth, annotationRect.size.width, CSSUnitType::CSS_PX);
Expand All @@ -133,4 +134,4 @@

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
10 changes: 7 additions & 3 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class PDFPluginBase : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<PDF
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(PDFPluginBase);
friend class PDFIncrementalLoader;
friend class PDFPluginChoiceAnnotation;
friend class PDFPluginTextAnnotation;
public:
static WebCore::PluginInfo pluginInfo();

Expand Down Expand Up @@ -120,8 +122,8 @@ class PDFPluginBase : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<PDF
virtual RefPtr<WebCore::ShareableBitmap> snapshot() { return nullptr; }
virtual void paint(WebCore::GraphicsContext&, const WebCore::IntRect&) { }

virtual CGFloat scaleFactor() const = 0;
virtual float contentScaleFactor() const = 0;
virtual double scaleFactor() const = 0;
virtual void setPageScaleFactor(double, std::optional<WebCore::IntPoint> origin) = 0;

virtual CGFloat minScaleFactor() const { return 0.25; }
virtual CGFloat maxScaleFactor() const { return 5; }
Expand All @@ -138,7 +140,6 @@ class PDFPluginBase : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<PDF
bool handlesPageScaleFactor() const;
virtual void didBeginMagnificationGesture() { }
virtual void didEndMagnificationGesture() { }
virtual void setPageScaleFactor(double, std::optional<WebCore::IntPoint> origin) = 0;

void updateControlTints(WebCore::GraphicsContext&);

Expand Down Expand Up @@ -255,6 +256,9 @@ class PDFPluginBase : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<PDF

uint64_t streamedBytes() const;

protected:
virtual double contentScaleFactor() const = 0;

private:
bool documentFinishedLoading() const { return m_documentFinishedLoading; }
void ensureDataBufferLength(uint64_t) WTF_REQUIRES_LOCK(m_streamedDataLock);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@
NSAttributedString *nsAttributedString = selection.attributedString;
RetainPtr scaledNSAttributedString = adoptNS([[NSMutableAttributedString alloc] initWithString:[nsAttributedString string]]);
NSFontManager *fontManager = [NSFontManager sharedFontManager];
CGFloat scaleFactor = contentScaleFactor();
auto scaleFactor = contentScaleFactor();

[nsAttributedString enumerateAttributesInRange:NSMakeRange(0, [nsAttributedString length]) options:0 usingBlock:^(NSDictionary *attributes, NSRange range, BOOL *stop) {
RetainPtr<NSMutableDictionary> scaledAttributes = adoptNS([attributes mutableCopy]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

#pragma once

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#include "PDFPluginAnnotation.h"

Expand Down Expand Up @@ -59,4 +59,4 @@ ALLOW_DEPRECATED_DECLARATIONS_END

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#if ENABLE(LEGACY_PDFKIT_PLUGIN)

#import "config.h"
#import "PDFPluginChoiceAnnotation.h"

#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#import <WebCore/CSSPrimitiveValue.h>
#import <WebCore/CSSPropertyNames.h>
#import <WebCore/ColorMac.h>
Expand Down Expand Up @@ -94,4 +94,4 @@

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
9 changes: 3 additions & 6 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPluginPasswordField.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef PDFPluginPasswordField_h
#define PDFPluginPasswordField_h
#pragma once

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#include "PDFPluginTextAnnotation.h"

Expand Down Expand Up @@ -54,6 +53,4 @@ class PDFPluginPasswordField : public PDFPluginTextAnnotation {

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)

#endif // PDFPluginTextAnnotation_h
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#import "config.h"
#import "PDFPluginPasswordField.h"

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#import "PDFLayerControllerSPI.h"
#import "PDFPlugin.h"
Expand Down Expand Up @@ -84,4 +84,4 @@

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef PDFPluginTextAnnotation_h
#define PDFPluginTextAnnotation_h
#pragma once

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#include "PDFPluginAnnotation.h"

Expand Down Expand Up @@ -69,6 +68,4 @@ ALLOW_DEPRECATED_DECLARATIONS_END

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)

#endif // PDFPluginTextAnnotation_h
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#import "config.h"
#import "PDFPluginTextAnnotation.h"

#if ENABLE(LEGACY_PDFKIT_PLUGIN)
#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)

#import "PDFAnnotationTextWidgetDetails.h"
#import "PDFLayerControllerSPI.h"
Expand Down Expand Up @@ -153,4 +153,4 @@ static const String cssAlignmentValueForNSTextAlignment(NSTextAlignment alignmen

} // namespace WebKit

#endif // ENABLE(LEGACY_PDFKIT_PLUGIN)
#endif // ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
PDFDataDetectorOverlayController& dataDetectorOverlayController() { return *m_dataDetectorOverlayController; }
#endif

float scaleForActualSize() const;
float initialScale() const;
float scaleForFitToView() const;
double scaleForActualSize() const;
double initialScale() const;
double scaleForFitToView() const;

/*
Unified PDF Plugin scales, in depth order:
Expand All @@ -218,8 +218,8 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
- "contentScaleFactor": the scale between the plugin and document space (scaleFactor * document layout scale)
*/
CGFloat scaleFactor() const override;
float contentScaleFactor() const final;
double scaleFactor() const override;
double contentScaleFactor() const final;

void didBeginMagnificationGesture() override;
void didEndMagnificationGesture() override;
Expand Down Expand Up @@ -502,7 +502,7 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay

WebCore::ScrollingNodeID m_scrollingNodeID;

float m_scaleFactor { 1 };
double m_scaleFactor { 1 };
bool m_inMagnificationGesture { false };
std::optional<WebCore::IntPoint> m_magnificationOriginInContentCoordinates;
std::optional<WebCore::IntPoint> m_magnificationOriginInPluginCoordinates;
Expand Down
20 changes: 10 additions & 10 deletions Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ - (void)formChanged:(NSNotification *)notification

// FIXME: We should rationalize these with the values in ViewGestureController.
// For now, we'll leave them differing as they do in PDFPlugin.
static constexpr CGFloat minimumZoomScale = 0.2;
static constexpr CGFloat maximumZoomScale = 6.0;
static constexpr CGFloat zoomIncrement = 1.18920;
static constexpr double minimumZoomScale = 0.2;
static constexpr double maximumZoomScale = 6.0;
static constexpr double zoomIncrement = 1.18920;

namespace WebKit {
using namespace WebCore;
Expand Down Expand Up @@ -979,7 +979,7 @@ static String mutationObserverNotificationString()
return pageIndex;
}

float UnifiedPDFPlugin::scaleForActualSize() const
double UnifiedPDFPlugin::scaleForActualSize() const
{
#if PLATFORM(MAC)
if (!m_frame || !m_frame->coreLocalFrame())
Expand Down Expand Up @@ -1009,7 +1009,7 @@ static String mutationObserverNotificationString()
return 1;
}

float UnifiedPDFPlugin::scaleForFitToView() const
double UnifiedPDFPlugin::scaleForFitToView() const
{
auto contentsSize = m_documentLayout.scaledContentsSize();
auto availableSize = FloatSize { availableContentsRect().size() };
Expand All @@ -1021,22 +1021,22 @@ static String mutationObserverNotificationString()
return aspectRatioFitRect.width() / size().width();
}

float UnifiedPDFPlugin::initialScale() const
double UnifiedPDFPlugin::initialScale() const
{
auto actualSizeScale = scaleForActualSize();
auto fitToViewScale = scaleForFitToView();
auto initialScale = std::max(actualSizeScale, fitToViewScale);
// Only let actual size scaling scale down, not up.
initialScale = std::min(initialScale, 1.0f);
initialScale = std::min(initialScale, 1.0);
return initialScale;
}

CGFloat UnifiedPDFPlugin::scaleFactor() const
double UnifiedPDFPlugin::scaleFactor() const
{
return m_scaleFactor;
}

float UnifiedPDFPlugin::contentScaleFactor() const
double UnifiedPDFPlugin::contentScaleFactor() const
{
return m_scaleFactor * m_documentLayout.scale();
}
Expand Down Expand Up @@ -1224,7 +1224,7 @@ static String mutationObserverNotificationString()
std::floor(std::max<float>(availableSize.height() - documentPresentationSize.height(), 0) / 2)
};

offset.scale(1.0f / m_scaleFactor);
offset.scale(1 / m_scaleFactor);
return offset;
}

Expand Down
5 changes: 0 additions & 5 deletions Source/WebKit/WebProcess/Plugins/PluginView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1004,11 +1004,6 @@ WebCore::FloatRect PluginView::rectForSelectionInRootView(PDFSelection *selectio
return protectedPlugin()->rectForSelectionInRootView(selection);
}

CGFloat PluginView::contentScaleFactor() const
{
return protectedPlugin()->contentScaleFactor();
}

bool PluginView::isUsingUISideCompositing() const
{
return protectedWebPage()->isUsingUISideCompositing();
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/WebProcess/Plugins/PluginView.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ class PluginView final : public WebCore::PluginViewBase {
bool performImmediateActionHitTestAtLocation(const WebCore::FloatPoint&, WebHitTestResultData&) const;

WebCore::FloatRect rectForSelectionInRootView(PDFSelection *) const;
CGFloat contentScaleFactor() const;

bool isUsingUISideCompositing() const;

Expand Down
5 changes: 4 additions & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2416,8 +2416,10 @@ void WebPage::setTextZoomFactor(double zoomFactor)
double WebPage::pageZoomFactor() const
{
#if ENABLE(PDF_PLUGIN)
if (auto* pluginView = mainFramePlugIn())
if (auto* pluginView = mainFramePlugIn()) {
// Note that this maps page *scale* factor to page *zoom* factor.
return pluginView->pageScaleFactor();
}
#endif

RefPtr frame = m_mainFrame->coreLocalFrame();
Expand All @@ -2430,6 +2432,7 @@ void WebPage::setPageZoomFactor(double zoomFactor)
{
#if ENABLE(PDF_PLUGIN)
if (auto* pluginView = mainFramePlugIn()) {
// Note that this maps page *zoom* factor to page *scale* factor.
pluginView->setPageScaleFactor(zoomFactor, std::nullopt);
return;
}
Expand Down

0 comments on commit 7706664

Please sign in to comment.