PDF SUBFRAMES: Incomplete repaint after pinch to zoom
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2011 19:01:51 +0000 (19:01 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2011 19:01:51 +0000 (19:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=70821
<rdar://problem/10312733>

Reviewed by Simon Fraser.

Add a new pure virtual member function, Plugin::wantsWindowRelativeCoordinates.
If a plug-in subclass returns true, we'll keep giving the plug-in coordinates that
are relative to the containing window. If a plug-in subclass returns false, we'll give it
coordinates in a much more sane coordinate system, with the origin at the top left corner of the plug-in.

Change BuiltinPDFView to return false so that it'll work correctly with pinch to zoom.

* WebProcess/Plugins/Netscape/NetscapePlugin.cpp:
(WebKit::NetscapePlugin::wantsWindowRelativeCoordinates):
Make wantsWindowRelativeCoordinates return true.

* WebProcess/Plugins/PDF/BuiltInPDFView.cpp:
(WebKit::BuiltInPDFView::paint):
Remove translation since the graphics context is already set up in the right way.

(WebKit::BuiltInPDFView::paintContent):
No need to offset by the plug-in view location anymore.

(WebKit::BuiltInPDFView::paintControls):
Account for the scrollbars being children of the parent scroll view here.

(WebKit::BuiltInPDFView::wantsWindowRelativeCoordinates):
Return false.

(WebKit::BuiltInPDFView::convertFromContainingViewToScrollbar):
Implement this so that scrollbar hit testing works correctly.

* WebProcess/Plugins/Plugin.h:
Add wantsWindowRelativeCoordinates.

* WebProcess/Plugins/PluginProxy.cpp:
(WebKit::PluginProxy::wantsWindowRelativeCoordinates):
Return true for now.

* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::renderer):
Add simple getter.

(WebKit::PluginView::paint):
Handle the case when the plug-in doesn't want window relative coordinates.

(WebKit::PluginView::transformsAffectFrameRect):
Return true here.

(WebKit::PluginView::viewGeometryDidChange):
No need to adjust the bounds to account for the scale factor now, since the frame rect will always be the
same regardless of the transform.

(WebKit::PluginView::clipRectInWindowCoordinates):
Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@98369 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h
Source/WebKit2/WebProcess/Plugins/Plugin.h
Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp
Source/WebKit2/WebProcess/Plugins/PluginProxy.h
Source/WebKit2/WebProcess/Plugins/PluginView.cpp
Source/WebKit2/WebProcess/Plugins/PluginView.h

index 34b26fc..50f7718 100644 (file)
@@ -1,3 +1,62 @@
+2011-10-25  Anders Carlsson  <andersca@apple.com>
+
+        PDF SUBFRAMES: Incomplete repaint after pinch to zoom
+        https://bugs.webkit.org/show_bug.cgi?id=70821
+        <rdar://problem/10312733>
+
+        Reviewed by Simon Fraser.
+
+        Add a new pure virtual member function, Plugin::wantsWindowRelativeCoordinates.
+        If a plug-in subclass returns true, we'll keep giving the plug-in coordinates that
+        are relative to the containing window. If a plug-in subclass returns false, we'll give it
+        coordinates in a much more sane coordinate system, with the origin at the top left corner of the plug-in.
+
+        Change BuiltinPDFView to return false so that it'll work correctly with pinch to zoom.
+        
+        * WebProcess/Plugins/Netscape/NetscapePlugin.cpp:
+        (WebKit::NetscapePlugin::wantsWindowRelativeCoordinates):
+        Make wantsWindowRelativeCoordinates return true.
+
+        * WebProcess/Plugins/PDF/BuiltInPDFView.cpp:
+        (WebKit::BuiltInPDFView::paint):
+        Remove translation since the graphics context is already set up in the right way.
+
+        (WebKit::BuiltInPDFView::paintContent):
+        No need to offset by the plug-in view location anymore.
+
+        (WebKit::BuiltInPDFView::paintControls):
+        Account for the scrollbars being children of the parent scroll view here.
+
+        (WebKit::BuiltInPDFView::wantsWindowRelativeCoordinates):
+        Return false.
+
+        (WebKit::BuiltInPDFView::convertFromContainingViewToScrollbar):
+        Implement this so that scrollbar hit testing works correctly.
+
+        * WebProcess/Plugins/Plugin.h:
+        Add wantsWindowRelativeCoordinates.
+
+        * WebProcess/Plugins/PluginProxy.cpp:
+        (WebKit::PluginProxy::wantsWindowRelativeCoordinates):
+        Return true for now.
+
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::renderer):
+        Add simple getter.
+
+        (WebKit::PluginView::paint):
+        Handle the case when the plug-in doesn't want window relative coordinates.
+
+        (WebKit::PluginView::transformsAffectFrameRect):
+        Return true here.
+
+        (WebKit::PluginView::viewGeometryDidChange):
+        No need to adjust the bounds to account for the scale factor now, since the frame rect will always be the
+        same regardless of the transform.
+
+        (WebKit::PluginView::clipRectInWindowCoordinates):
+        Ditto.
+
 2011-10-25  John Sullivan  <sullivan@apple.com>
 
         "Open with" item missing from PDF context menu in some cases
index db2ceb7..4d87985 100644 (file)
@@ -891,6 +891,11 @@ bool NetscapePlugin::handleScroll(ScrollDirection, ScrollGranularity)
     return false;
 }
 
+bool NetscapePlugin::wantsWindowRelativeCoordinates()
+{
+    return true;
+}
+
 Scrollbar* NetscapePlugin::horizontalScrollbar()
 {
     return 0;
index d53842b..84f66fc 100644 (file)
@@ -207,6 +207,7 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual bool wantsWindowRelativeCoordinates();
     virtual WebCore::Scrollbar* horizontalScrollbar();
     virtual WebCore::Scrollbar* verticalScrollbar();
 
index 88d7ff5..8ab79cd 100644 (file)
@@ -38,6 +38,7 @@
 #include <WebCore/LocalizedStrings.h>
 #include <WebCore/Page.h>
 #include <WebCore/PluginData.h>
+#include <WebCore/RenderBoxModelObject.h>
 #include <WebCore/ScrollAnimator.h>
 #include <WebCore/ScrollbarTheme.h>
 
@@ -232,22 +233,15 @@ void BuiltInPDFView::destroy()
     destroyScrollbar(VerticalScrollbar);
 }
 
-void BuiltInPDFView::paint(GraphicsContext* graphicsContext, const IntRect& dirtyRectInWindowCoordinates)
+void BuiltInPDFView::paint(GraphicsContext* graphicsContext, const IntRect& dirtyRect)
 {
     scrollAnimator()->contentAreaWillPaint();
 
-    paintBackground(graphicsContext, dirtyRectInWindowCoordinates);
+    paintBackground(graphicsContext, dirtyRect);
 
     if (!m_pdfDocument) // FIXME: Draw loading progress.
         return;
 
-    GraphicsContextStateSaver stateSaver(*graphicsContext);
-
-    // Undo translation to window coordinates performed by PluginView::paint().
-    IntRect dirtyRect = pluginView()->parent()->windowToContents(dirtyRectInWindowCoordinates);
-    IntPoint documentOriginInWindowCoordinates = pluginView()->parent()->windowToContents(IntPoint());
-    graphicsContext->translate(-documentOriginInWindowCoordinates.x(), -documentOriginInWindowCoordinates.y());
-
     paintContent(graphicsContext, dirtyRect);
     paintControls(graphicsContext, dirtyRect);
 }
@@ -271,9 +265,8 @@ void BuiltInPDFView::paintContent(GraphicsContext* graphicsContext, const IntRec
 
     graphicsContext->clip(dirtyRect);
     IntRect contentRect(dirtyRect);
-    contentRect.moveBy(-pluginView()->location());
     contentRect.moveBy(IntPoint(m_scrollOffset));
-    graphicsContext->translate(pluginView()->x() - m_scrollOffset.width(), pluginView()->y() - m_scrollOffset.height());
+    graphicsContext->translate(-m_scrollOffset.width(), -m_scrollOffset.height());
 
     CGContextScaleCTM(context, 1, -1);
 
@@ -307,10 +300,18 @@ void BuiltInPDFView::paintContent(GraphicsContext* graphicsContext, const IntRec
 
 void BuiltInPDFView::paintControls(GraphicsContext* graphicsContext, const IntRect& dirtyRect)
 {
-    if (m_horizontalScrollbar)
-        m_horizontalScrollbar->paint(graphicsContext, dirtyRect);
-    if (m_verticalScrollbar)
-        m_verticalScrollbar->paint(graphicsContext, dirtyRect);
+    {
+        GraphicsContextStateSaver stateSaver(*graphicsContext);
+        IntRect scrollbarDirtyRect = dirtyRect;
+        scrollbarDirtyRect.moveBy(pluginView()->frameRect().location());
+        graphicsContext->translate(-pluginView()->frameRect().x(), -pluginView()->frameRect().y());
+
+        if (m_horizontalScrollbar)
+            m_horizontalScrollbar->paint(graphicsContext, scrollbarDirtyRect);
+
+        if (m_verticalScrollbar)
+            m_verticalScrollbar->paint(graphicsContext, scrollbarDirtyRect);
+    }
 
     IntRect dirtyCornerRect = intersection(scrollCornerRect(), dirtyRect);
     ScrollbarTheme::theme()->paintScrollCorner(0, graphicsContext, dirtyCornerRect);
@@ -531,6 +532,11 @@ bool BuiltInPDFView::handleScroll(ScrollDirection direction, ScrollGranularity g
     return scroll(direction, granularity);
 }
 
+bool BuiltInPDFView::wantsWindowRelativeCoordinates()
+{
+    return false;
+}
+
 Scrollbar* BuiltInPDFView::horizontalScrollbar()
 {
     return m_horizontalScrollbar.get();
@@ -656,4 +662,12 @@ void BuiltInPDFView::scrollbarStyleChanged()
     scrollAnimator()->contentsResized();
 }
 
+IntPoint BuiltInPDFView::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntPoint& parentPoint) const
+{
+    IntPoint point = pluginView()->frame()->view()->convertToRenderer(pluginView()->renderer(), parentPoint);
+    point.move(pluginView()->location() - scrollbar->location());
+
+    return point;
+}
+
 } // namespace WebKit
index 62d30ec..b31525d 100644 (file)
@@ -111,6 +111,7 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual bool wantsWindowRelativeCoordinates();
     virtual WebCore::Scrollbar* horizontalScrollbar();
     virtual WebCore::Scrollbar* verticalScrollbar();
 
@@ -137,6 +138,9 @@ private:
     virtual bool shouldSuspendScrollAnimations() const { return false; } // If we return true, ScrollAnimatorMac will keep cycling a timer forever, waiting for a good time to animate.
     virtual void scrollbarStyleChanged();
 
+    // FIXME: Implement the other conversion functions; this one is enough to get scrollbar hit testing working.
+    virtual WebCore::IntPoint convertFromContainingViewToScrollbar(const WebCore::Scrollbar*, const WebCore::IntPoint& parentPoint) const;
+
     // In window coordinates.
     WebCore::IntRect m_frameRect;
 
index cab6661..bca7475 100644 (file)
@@ -207,6 +207,10 @@ public:
     // Tells the plug-in that it should scroll. The plug-in should return true if it did scroll.
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity) = 0;
 
+    // Whether the plug-in wants its coordinates to be relative to the window.
+    // FIXME: No plug-ins should want window relative coordinates, so we should get rid of this.
+    virtual bool wantsWindowRelativeCoordinates() = 0;
+
     // A plug-in can use WebCore scroll bars. Make them known, so that hit testing can find them.
     // FIXME: This code should be in PluginView or its base class, not in individual plug-ins.
     virtual WebCore::Scrollbar* horizontalScrollbar() = 0;
index 28f1cbe..9684c60 100644 (file)
@@ -397,6 +397,11 @@ bool PluginProxy::handleScroll(ScrollDirection, ScrollGranularity)
     return false;
 }
 
+bool PluginProxy::wantsWindowRelativeCoordinates()
+{
+    return true;
+}
+
 Scrollbar* PluginProxy::horizontalScrollbar()
 {
     return 0;
index a5437b1..0dd8edd 100644 (file)
@@ -104,6 +104,7 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual bool wantsWindowRelativeCoordinates();
     virtual WebCore::Scrollbar* horizontalScrollbar();
     virtual WebCore::Scrollbar* verticalScrollbar();
 
index 2e65674..c22a802 100644 (file)
@@ -382,6 +382,11 @@ void PluginView::manualLoadDidFail(const ResourceError& error)
     m_plugin->manualStreamDidFail(error.isCancellation());
 }
 
+RenderBoxModelObject* PluginView::renderer() const
+{
+    return toRenderBoxModelObject(m_pluginElement->renderer());
+}
+
 #if PLATFORM(MAC)    
 void PluginView::setWindowIsVisible(bool windowIsVisible)
 {
@@ -563,24 +568,38 @@ void PluginView::paint(GraphicsContext* context, const IntRect& dirtyRect)
         return;
     }
 
-    IntRect dirtyRectInWindowCoordinates = parent()->contentsToWindow(dirtyRect);
-    IntRect paintRectInWindowCoordinates = intersection(dirtyRectInWindowCoordinates, clipRectInWindowCoordinates());
-    if (paintRectInWindowCoordinates.isEmpty())
+    IntRect paintRect;
+    if (m_plugin->wantsWindowRelativeCoordinates()) {
+        IntRect dirtyRectInWindowCoordinates = parent()->contentsToWindow(dirtyRect);
+        paintRect = intersection(dirtyRectInWindowCoordinates, clipRectInWindowCoordinates());
+    } else {
+        // FIXME: We should try to intersect the dirty rect with the plug-in's clip rect here.
+        paintRect = IntRect(IntPoint(), frameRect().size());
+    }
+
+    if (paintRect.isEmpty())
         return;
 
-    if (m_snapshot)
+    if (m_snapshot) {
         m_snapshot->paint(*context, frameRect().location(), m_snapshot->bounds());
-    else {
+        return;
+    }
+    
+    GraphicsContextStateSaver stateSaver(*context);
+
+    if (m_plugin->wantsWindowRelativeCoordinates()) {
         // The plugin is given a frame rect which is parent()->contentsToWindow(frameRect()),
         // and un-translates by the its origin when painting. The current CTM reflects
         // this widget's frame is its parent (the document), so we have to offset the CTM by
         // the document's window coordinates.
         IntPoint documentOriginInWindowCoordinates = parent()->contentsToWindow(IntPoint());
-        
-        GraphicsContextStateSaver stateSaver(*context);
         context->translate(-documentOriginInWindowCoordinates.x(), -documentOriginInWindowCoordinates.y());
-        m_plugin->paint(context, paintRectInWindowCoordinates);
+    } else {
+        // Translate the coordinate system so that the origin is in the top-left corner of the plug-in.
+        context->translate(frameRect().location().x(), frameRect().location().y());
     }
+
+    m_plugin->paint(context, paintRect);
 }
 
 void PluginView::frameRectsChanged()
@@ -679,16 +698,19 @@ void PluginView::hide()
     Widget::hide();
 }
 
+bool PluginView::transformsAffectFrameRect()
+{
+    return false;
+}
+
 void PluginView::viewGeometryDidChange()
 {
     if (!m_isInitialized || !m_plugin || !parent())
         return;
 
     // Get the frame rect in window coordinates.
+    // FIXME: Figure out what we should pass here when m_plugin->wantsWindowRelativeCoordinates() returns false.
     IntRect frameRectInWindowCoordinates = parent()->contentsToWindow(frameRect());
-    
-    // Adjust bounds to account for frameScaleFactor
-    frameRectInWindowCoordinates.scale(1 / frame()->frameScaleFactor());
     m_plugin->geometryDidChange(frameRectInWindowCoordinates, clipRectInWindowCoordinates());
 }
 
@@ -714,7 +736,6 @@ IntRect PluginView::clipRectInWindowCoordinates() const
     // Intersect the two rects to get the view clip rect in window coordinates.
     frameRectInWindowCoordinates.intersect(windowClipRect);
 
-    frameRectInWindowCoordinates.scale(1 / frame->frameScaleFactor());
     return frameRectInWindowCoordinates;
 }
 
index ce196b8..56acecb 100644 (file)
@@ -67,6 +67,9 @@ public:
     bool sendComplexTextInput(uint64_t pluginComplexTextInputIdentifier, const String& textInput);
 #endif
 
+    // FIXME: Remove this; nobody should have to know about the plug-in view's renderer except the plug-in view itself.
+    WebCore::RenderBoxModelObject* renderer() const;
+
 private:
     PluginView(PassRefPtr<WebCore::HTMLPlugInElement>, PassRefPtr<Plugin>, const Plugin::Parameters& parameters);
     virtual ~PluginView();
@@ -118,6 +121,7 @@ private:
     virtual void notifyWidget(WebCore::WidgetNotification);
     virtual void show();
     virtual void hide();
+    virtual bool transformsAffectFrameRect();
 
     // WebCore::MediaCanStartListener
     virtual void mediaCanStart();