Changeset 53637 in webkit

Timestamp:

Jan 21, 2010 12:19:10 PM (14 years ago)

Author:

Simon Fraser

Message:

2010-01-20 Simon Fraser <Simon Fraser>

Reviewed by Dave Hyatt.

Hit testing on composited plugins is broken
​https://bugs.webkit.org/show_bug.cgi?id=33927
<rdar://problem/7559069>

RenderWidget::paint()'s strategy of moving widgets at paint time, using tx and ty, was flawed
because tx,ty are not always root-relative, especially when painting into compositing layers.
This would move widgets to the wrong location, which caused hit testing issues.

Widgets are usually positioned by layout. The one time this was not true was scrolling fixed-position
elements, so we now reposition widgets after scrolling too.

There was a related problem, which was that widgets expect the graphics context to be set up for
root-relative painting. To fix this, adjust the CTM and the paintRect when the widget's frameRect
is in a different coordinate system to the painting offset.

Test: plugins/mouse-events-fixedpos.html

• page/FrameView.cpp: (WebCore::FrameView::scrollPositionChanged): Update widget positions, to handle widgets in fixed position elements, but only if we're not already inside of layout.

• platform/graphics/GraphicsContext.h: (WebCore::GraphicsContext::translate): Add a translate() convenience method that takes a FloatSize.

• platform/graphics/IntSize.h: (WebCore::IntSize::isZero): Add a convenience method for testing for a zero size.

• platform/mac/WidgetMac.mm: (WebCore::Widget::paint): Adjust a comment.

• rendering/RenderWidget.cpp: (WebCore::RenderWidget::paint): Detect when the widget's frame is in a different coordinate system to painting, and adjust the CTM and paintRect in that case.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/plugins/mouse-events-fixedpos-expected.txt (added)
• LayoutTests/plugins/mouse-events-fixedpos.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (1 diff)
• WebCore/platform/graphics/GraphicsContext.h (modified) (1 diff)
• WebCore/platform/graphics/IntSize.h (modified) (1 diff)
• WebCore/platform/mac/WidgetMac.mm (modified) (1 diff)
• WebCore/rendering/RenderWidget.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-01-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dave Hyatt.
+
+        Hit testing on composited plugins is broken
+        https://bugs.webkit.org/show_bug.cgi?id=33927
+        <rdar://problem/7559069>
+
+        Testcase for hit testing on a fixed-position plugin after scrolling.
+        
+        * plugins/mouse-events-fixedpos-expected.txt: Added.
+        * plugins/mouse-events-fixedpos.html: Added.
+
 2010-01-21  Diego Gonzalez  <diego.gonzalez@openbossa.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dave Hyatt.
+
+        Hit testing on composited plugins is broken
+        https://bugs.webkit.org/show_bug.cgi?id=33927
+        <rdar://problem/7559069>
+
+        RenderWidget::paint()'s strategy of moving widgets at paint time, using tx and ty, was flawed
+        because tx,ty are not always root-relative, especially when painting into compositing layers.
+        This would move widgets to the wrong location, which caused hit testing issues.
+        
+        Widgets are usually positioned by layout. The one time this was not true was scrolling fixed-position
+        elements, so we now reposition widgets after scrolling too.
+        
+        There was a related problem, which was that widgets expect the graphics context to be set up for
+        root-relative painting. To fix this, adjust the CTM and the paintRect when the widget's frameRect
+        is in a different coordinate system to the painting offset.
+        
+        Test: plugins/mouse-events-fixedpos.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::scrollPositionChanged): Update widget positions, to handle widgets in fixed position
+        elements, but only if we're not already inside of layout.
+
+        * platform/graphics/GraphicsContext.h:
+        (WebCore::GraphicsContext::translate): Add a translate() convenience method that takes a FloatSize.
+
+        * platform/graphics/IntSize.h:
+        (WebCore::IntSize::isZero): Add a convenience method for testing for a zero size.
+
+        * platform/mac/WidgetMac.mm:
+        (WebCore::Widget::paint): Adjust a comment.
+
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::paint): Detect when the widget's frame is in a different coordinate system
+        to painting, and adjust the CTM and paintRect in that case.
+
 2010-01-21  Andrei Popescu  <andreip@google.com>
 

trunk/WebCore/page/FrameView.cpp

@@ -951 +951 @@
         layer->updateLayerPositions(RenderLayer::UpdateCompositingLayers);
 #endif
+
+    // Update widget positions to take care of widgets inside fixed position elements,
+    // but only if we're not inside of layout.
+    if (!m_nestedLayoutCount) {
+        if (RenderView* root = m_frame->contentRenderer())
+            root->updateWidgetPositions();
+    }
 }
 

trunk/WebCore/platform/graphics/GraphicsContext.h

@@ -292 +292 @@
         void scale(const FloatSize&);
         void rotate(float angleInRadians);
+        void translate(const FloatSize& size) { translate(size.width(), size.height()); }
         void translate(float x, float y);
         IntPoint origin();

trunk/WebCore/platform/graphics/IntSize.h

@@ -70 +70 @@
 
     bool isEmpty() const { return m_width <= 0 || m_height <= 0; }
-
+    bool isZero() const { return !m_width && !m_height; }
+    
     void expand(int width, int height)
     {

trunk/WebCore/platform/mac/WidgetMac.mm

@@ -198 +198 @@
     } else {
         // This is the case of drawing into a bitmap context other than a window backing store. It gets hit beneath
-        // -cacheDisplayInRect:toBitmapImageRep:.
+        // -cacheDisplayInRect:toBitmapImageRep:, and when painting into compositing layers.
 
         // Transparent subframes are in fact implemented with scroll views that return YES from -drawsBackground (whenever the WebView

trunk/WebCore/rendering/RenderWidget.cpp

@@ -253 +253 @@
 
     if (m_widget) {
-        // Move the widget if necessary.  We normally move and resize widgets during layout, but sometimes
-        // widgets can move without layout occurring (most notably when you scroll a document that
-        // contains fixed positioned elements).
-        m_widget->move(tx + borderLeft() + paddingLeft(), ty + borderTop() + paddingTop());
-
         // Tell the widget to paint now.  This is the only time the widget is allowed
         // to paint itself.  That way it will composite properly with z-indexed layers.
         if (m_substituteImage)
             paintInfo.context->drawImage(m_substituteImage.get(), style()->colorSpace(), m_widget->frameRect());
-        else
-            m_widget->paint(paintInfo.context, paintInfo.rect);
-
+        else {
+            IntPoint widgetLocation = m_widget->frameRect().location();
+            IntPoint paintLocation(tx + borderLeft() + paddingLeft(), ty + borderTop() + paddingTop());
+            IntRect paintRect = paintInfo.rect;
+
+            IntSize paintOffset = paintLocation - widgetLocation;
+            // When painting widgets into compositing layers, tx and ty are relative to the enclosing compositing layer,
+            // not the root. In this case, shift the CTM and adjust the paintRect to be root-relative to fix plug-in drawing.
+            if (!paintOffset.isZero()) {
+                paintInfo.context->translate(paintOffset);
+                paintRect.move(-paintOffset);
+            }
+            m_widget->paint(paintInfo.context, paintRect);
+
+            if (!paintOffset.isZero())
+                paintInfo.context->translate(-paintOffset);
+        }
         if (m_widget->isFrameView() && paintInfo.overlapTestRequests && !static_cast<FrameView*>(m_widget.get())->useSlowRepaintsIfNotOverlapped()) {
             ASSERT(!paintInfo.overlapTestRequests->contains(this));