From d1e45374111c047fc69c8719434bf14417f74d2e Mon Sep 17 00:00:00 2001 From: pdr Date: Fri, 26 Feb 2016 15:24:02 -0800 Subject: Update drag images to paint from the nearest self painting layer Before this patch there were two codepaths for painting drag images: 1) Boxes were drawn using paintAsPseudoStackingContext which would fail to paint composited children (https://crbug.com/585611). 2) Everything else was drawn using paintIntoDragImage which paints starting at the view so any transparent regions of the drag element would be filled in with the content behind the drag element. This patch unifies drag image rendering under a single approach where painting begins at the nearest self painting layer to the dragged element. This has two nice properties: 1) self painting children are painted. 2) transparency is supported as long as the drag element has a layer. Neither Safari nor Firefox really support these usecases so this approach is a best-effort at supporting common usecases without invasive surgery on the paint code. Some behavior is different such as not including the dragged element's transform. As part of this patch, paintIntoDragImage has been merged into dragImageForSelection which is now the only caller. BUG=585611 Review URL: https://codereview.chromium.org/1736893002 Cr-Commit-Position: refs/heads/master@{#378021} --- .../WebKit/Source/core/frame/LocalFrame.cpp | 53 +++++++++------------- third_party/WebKit/Source/core/frame/LocalFrame.h | 5 -- .../WebKit/Source/core/paint/PaintLayer.cpp | 8 ++++ third_party/WebKit/Source/core/paint/PaintLayer.h | 3 ++ .../WebKit/Source/web/tests/WebFrameTest.cpp | 11 ++++- .../WebKit/Source/web/tests/data/nodeimage.html | 7 +++ 6 files changed, 49 insertions(+), 38 deletions(-) diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/third_party/WebKit/Source/core/frame/LocalFrame.cpp index c66840f..ca6138a 100644 --- a/third_party/WebKit/Source/core/frame/LocalFrame.cpp +++ b/third_party/WebKit/Source/core/frame/LocalFrame.cpp @@ -97,7 +97,7 @@ namespace { class DragImageBuilder { STACK_ALLOCATED(); public: - DragImageBuilder(const LocalFrame* localFrame, const IntRect& bounds, Node* draggedNode, float opacity = 1) + DragImageBuilder(const LocalFrame* localFrame, const FloatRect& bounds, Node* draggedNode, float opacity = 1) : m_localFrame(localFrame) , m_draggedNode(draggedNode) , m_bounds(bounds) @@ -140,7 +140,7 @@ public: private: RawPtrWillBeMember m_localFrame; RawPtrWillBeMember m_draggedNode; - IntRect m_bounds; + FloatRect m_bounds; float m_opacity; OwnPtr m_pictureBuilder; }; @@ -631,18 +631,6 @@ double LocalFrame::devicePixelRatio() const return ratio; } -PassOwnPtr LocalFrame::paintIntoDragImage(const GlobalPaintFlags globalPaintFlags, - IntRect paintingRect, Node* draggedNode, float opacity) -{ - ASSERT(document()->isActive()); - // Not flattening compositing layers will result in a broken image being painted. - ASSERT(globalPaintFlags & GlobalPaintFlattenCompositingLayers); - - DragImageBuilder dragImageBuilder(this, paintingRect, draggedNode, opacity); - m_view->paintContents(dragImageBuilder.context(), globalPaintFlags, paintingRect); - return dragImageBuilder.createImage(); -} - PassOwnPtr LocalFrame::nodeImage(Node& node) { m_view->updateAllLifecyclePhases(); @@ -650,23 +638,20 @@ PassOwnPtr LocalFrame::nodeImage(Node& node) if (!layoutObject) return nullptr; - // Directly paint boxes as if they are a stacking context. - if (layoutObject->isBox() && layoutObject->container()) { - IntRect boundingBox = layoutObject->absoluteBoundingBoxRectIncludingDescendants(); - LayoutPoint paintOffset = boundingBox.location() - layoutObject->offsetFromContainer(layoutObject->container(), LayoutPoint()); - - DragImageBuilder dragImageBuilder(this, boundingBox, &node); - { - PaintInfo paintInfo(dragImageBuilder.context(), boundingBox, PaintPhase::PaintPhaseForeground, GlobalPaintFlattenCompositingLayers, 0); - ObjectPainter(*layoutObject).paintAsPseudoStackingContext(paintInfo, LayoutPoint(paintOffset)); - } - return dragImageBuilder.createImage(); + // Paint starting at the nearest self painting layer, clipped to the object itself. + // TODO(pdr): This will also paint the content behind the object if the object contains + // transparency but the layer is opaque. We could directly call layoutObject->paint(...) + // (see ObjectPainter::paintAsPseudoStackingContext) but this would skip self-painting children. + PaintLayer* layer = layoutObject->enclosingLayer()->enclosingSelfPaintingLayer(); + IntRect absoluteBoundingBox = layoutObject->absoluteBoundingBoxRectIncludingDescendants(); + FloatRect boundingBox = layer->layoutObject()->absoluteToLocalQuad(FloatQuad(absoluteBoundingBox), UseTransforms).boundingBox(); + DragImageBuilder dragImageBuilder(this, boundingBox, &node); + { + PaintLayerPaintingInfo paintingInfo(layer, LayoutRect(boundingBox), GlobalPaintFlattenCompositingLayers, LayoutSize(), 0); + PaintLayerFlags flags = PaintLayerHaveTransparency | PaintLayerAppliedTransform | PaintLayerUncachedClipRects; + PaintLayerPainter(*layer).paintLayer(dragImageBuilder.context(), paintingInfo, flags); } - - // TODO(pdr): This will also paint the background if the object contains transparency. We can - // directly call layoutObject->paint(...) (see: ObjectPainter::paintAsPseudoStackingContext) but - // painters are inconsistent about which transform space they expect (see: svg, inlines, etc.) - return paintIntoDragImage(GlobalPaintFlattenCompositingLayers, layoutObject->absoluteBoundingBoxRectIncludingDescendants(), &node); + return dragImageBuilder.createImage(); } PassOwnPtr LocalFrame::dragImageForSelection(float opacity) @@ -675,9 +660,13 @@ PassOwnPtr LocalFrame::dragImageForSelection(float opacity) return nullptr; m_view->updateAllLifecyclePhases(); + ASSERT(document()->isActive()); - return paintIntoDragImage(GlobalPaintSelectionOnly | GlobalPaintFlattenCompositingLayers, - enclosingIntRect(selection().bounds()), nullptr, opacity); + FloatRect paintingRect = FloatRect(selection().bounds()); + DragImageBuilder dragImageBuilder(this, paintingRect, nullptr, opacity); + GlobalPaintFlags paintFlags = GlobalPaintSelectionOnly | GlobalPaintFlattenCompositingLayers; + m_view->paintContents(dragImageBuilder.context(), paintFlags, enclosingIntRect(paintingRect)); + return dragImageBuilder.createImage(); } String LocalFrame::selectedText() const diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.h b/third_party/WebKit/Source/core/frame/LocalFrame.h index 9288d04..71067fb 100644 --- a/third_party/WebKit/Source/core/frame/LocalFrame.h +++ b/third_party/WebKit/Source/core/frame/LocalFrame.h @@ -199,11 +199,6 @@ private: String localLayerTreeAsText(unsigned flags) const; - // Paints the area for the given rect into a DragImage. - // The rect is in the coordinate space of the frame. - PassOwnPtr paintIntoDragImage(const GlobalPaintFlags, - IntRect paintingRect, Node* draggedNode = nullptr, float opacity = 1); - void enableNavigation() { --m_navigationDisableCount; } void disableNavigation() { ++m_navigationDisableCount; } diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.cpp b/third_party/WebKit/Source/core/paint/PaintLayer.cpp index 2a34031c..4ae5132 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayer.cpp +++ b/third_party/WebKit/Source/core/paint/PaintLayer.cpp @@ -2452,6 +2452,14 @@ void PaintLayer::updateSelfPaintingLayer() parent()->dirtyAncestorChainHasSelfPaintingLayerDescendantStatus(); } +PaintLayer* PaintLayer::enclosingSelfPaintingLayer() +{ + PaintLayer* layer = this; + while (layer && !layer->isSelfPaintingLayer()) + layer = layer->parent(); + return layer; +} + bool PaintLayer::hasNonEmptyChildLayoutObjects() const { // Some HTML can cause whitespace text nodes to have layoutObjects, like: diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.h b/third_party/WebKit/Source/core/paint/PaintLayer.h index 3468e4b..4749c70 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayer.h +++ b/third_party/WebKit/Source/core/paint/PaintLayer.h @@ -610,6 +610,9 @@ public: void updateOrRemoveFilterEffectBuilder(); void updateSelfPaintingLayer(); + // This is O(depth) so avoid calling this in loops. Instead use optimizations like + // those in PaintInvalidationState. + PaintLayer* enclosingSelfPaintingLayer(); PaintLayer* enclosingTransformedAncestor() const; LayoutPoint computeOffsetFromTransformedAncestor() const; diff --git a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp index 6626bf2..41c5d4c 100644 --- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp +++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp @@ -6899,6 +6899,15 @@ static void nodeImageTestValidation(const IntSize& referenceBitmapSize, DragImag EXPECT_EQ(0, memcmp(bitmap.getPixels(), dragBitmap.getPixels(), bitmap.getSize())); } +TEST_P(ParameterizedWebFrameTest, NodeImageTestCSSTransformDescendant) +{ + FrameTestHelpers::WebViewHelper webViewHelper(this); + OwnPtr dragImage = nodeImageTestSetup(&webViewHelper, std::string("case-css-3dtransform-descendant")); + EXPECT_TRUE(dragImage); + + nodeImageTestValidation(IntSize(40, 40), dragImage.get()); +} + TEST_P(ParameterizedWebFrameTest, NodeImageTestCSSTransform) { FrameTestHelpers::WebViewHelper webViewHelper(this); @@ -6914,7 +6923,7 @@ TEST_P(ParameterizedWebFrameTest, NodeImageTestCSS3DTransform) OwnPtr dragImage = nodeImageTestSetup(&webViewHelper, std::string("case-css-3dtransform")); EXPECT_TRUE(dragImage); - nodeImageTestValidation(IntSize(20, 40), dragImage.get()); + nodeImageTestValidation(IntSize(40, 40), dragImage.get()); } TEST_P(ParameterizedWebFrameTest, NodeImageTestInlineBlock) diff --git a/third_party/WebKit/Source/web/tests/data/nodeimage.html b/third_party/WebKit/Source/web/tests/data/nodeimage.html index 846a2db..37cf41f 100644 --- a/third_party/WebKit/Source/web/tests/data/nodeimage.html +++ b/third_party/WebKit/Source/web/tests/data/nodeimage.html @@ -1,5 +1,12 @@
+
+
+
+
+
+ +
-- cgit v1.1