From fa51f70e8b18f0c13c2031b43e14792e012bef58 Mon Sep 17 00:00:00 2001 From: "leviw@chromium.org" Date: Mon, 16 Apr 2012 23:24:15 +0000 Subject: [PATCH] Make borderBoxRect sub-pixel precise and add a pixel snapped version https://bugs.webkit.org/show_bug.cgi?id=84063 Reviewed by Eric Seidel. In an effort to prevent misuse, we previously decided to have borderBoxRect return a pixel-snapped IntRect. This is because borderBoxRect returns a rect that's positioned at (0,0), and therefore won't snap to the same size as the element it's covering. There are a couple uses of borderBoxRect that don't pixel snap the values and require sub-pixel precision. This patch adds a pixelSnappedBorderBoxRect that makes the snapping explicit, and moves uses that would otherwise pixel snap the rect to this version to avoid producing a rect of the incorrect size. For details about pixel snapping with LayoutUnits, please see https://trac.webkit.org/wiki/LayoutUnit No new tests. No change in behavior. * html/shadow/TextControlInnerElements.cpp: (WebCore::SpinButtonElement::defaultEventHandler): * rendering/RenderBlock.cpp: (WebCore::RenderBlock::addVisualOverflowFromTheme): * rendering/RenderBox.h: (WebCore::RenderBox::borderBoxRect): (WebCore::RenderBox::pixelSnappedBorderBoxRect): (WebCore::RenderBox::borderBoundingBox): (WebCore::RenderBox::hasVisualOverflow): * rendering/RenderLayer.cpp: (WebCore::RenderLayer::scrollCornerRect): (WebCore::RenderLayer::scrollCornerAndResizerRect): (WebCore::RenderLayer::horizontalScrollbarStart): (WebCore::RenderLayer::positionOverflowControls): (WebCore::RenderLayer::paintResizer): (WebCore::RenderLayer::hitTestOverflowControls): * rendering/RenderLayerBacking.cpp: (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): (WebCore::RenderLayerBacking::startAnimation): (WebCore::RenderLayerBacking::startTransition): * rendering/RenderTable.cpp: (WebCore::RenderTable::addOverflowFromChildren): * rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::paintSearchFieldCancelButton): (WebCore::RenderThemeMac::paintSearchFieldResultsDecoration): (WebCore::RenderThemeMac::paintSearchFieldResultsButton): * rendering/svg/RenderSVGRoot.cpp: (WebCore::RenderSVGRoot::paintReplaced): (WebCore::RenderSVGRoot::computeFloatRectForRepaint): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@114315 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 49 ++++++++++++++++++++++ .../html/shadow/TextControlInnerElements.cpp | 4 +- Source/WebCore/rendering/RenderBlock.cpp | 2 +- Source/WebCore/rendering/RenderBox.h | 7 ++-- Source/WebCore/rendering/RenderLayer.cpp | 12 +++--- Source/WebCore/rendering/RenderLayerBacking.cpp | 6 +-- Source/WebCore/rendering/RenderTable.cpp | 2 +- Source/WebCore/rendering/RenderThemeMac.mm | 6 +-- Source/WebCore/rendering/svg/RenderSVGRoot.cpp | 4 +- 9 files changed, 71 insertions(+), 21 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 41d1041..1b727a6 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,52 @@ +2012-04-16 Levi Weintraub + + Make borderBoxRect sub-pixel precise and add a pixel snapped version + https://bugs.webkit.org/show_bug.cgi?id=84063 + + Reviewed by Eric Seidel. + + In an effort to prevent misuse, we previously decided to have borderBoxRect return a + pixel-snapped IntRect. This is because borderBoxRect returns a rect that's positioned + at (0,0), and therefore won't snap to the same size as the element it's covering. + + There are a couple uses of borderBoxRect that don't pixel snap the values and require + sub-pixel precision. This patch adds a pixelSnappedBorderBoxRect that makes the snapping + explicit, and moves uses that would otherwise pixel snap the rect to this version to + avoid producing a rect of the incorrect size. For details about pixel snapping with + LayoutUnits, please see https://trac.webkit.org/wiki/LayoutUnit + + No new tests. No change in behavior. + + * html/shadow/TextControlInnerElements.cpp: + (WebCore::SpinButtonElement::defaultEventHandler): + * rendering/RenderBlock.cpp: + (WebCore::RenderBlock::addVisualOverflowFromTheme): + * rendering/RenderBox.h: + (WebCore::RenderBox::borderBoxRect): + (WebCore::RenderBox::pixelSnappedBorderBoxRect): + (WebCore::RenderBox::borderBoundingBox): + (WebCore::RenderBox::hasVisualOverflow): + * rendering/RenderLayer.cpp: + (WebCore::RenderLayer::scrollCornerRect): + (WebCore::RenderLayer::scrollCornerAndResizerRect): + (WebCore::RenderLayer::horizontalScrollbarStart): + (WebCore::RenderLayer::positionOverflowControls): + (WebCore::RenderLayer::paintResizer): + (WebCore::RenderLayer::hitTestOverflowControls): + * rendering/RenderLayerBacking.cpp: + (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): + (WebCore::RenderLayerBacking::startAnimation): + (WebCore::RenderLayerBacking::startTransition): + * rendering/RenderTable.cpp: + (WebCore::RenderTable::addOverflowFromChildren): + * rendering/RenderThemeMac.mm: + (WebCore::RenderThemeMac::paintSearchFieldCancelButton): + (WebCore::RenderThemeMac::paintSearchFieldResultsDecoration): + (WebCore::RenderThemeMac::paintSearchFieldResultsButton): + * rendering/svg/RenderSVGRoot.cpp: + (WebCore::RenderSVGRoot::paintReplaced): + (WebCore::RenderSVGRoot::computeFloatRectForRepaint): + 2012-04-16 Anders Carlsson Crash when running some editing related tests diff --git a/Source/WebCore/html/shadow/TextControlInnerElements.cpp b/Source/WebCore/html/shadow/TextControlInnerElements.cpp index 7bee1e6..ab318ea 100644 --- a/Source/WebCore/html/shadow/TextControlInnerElements.cpp +++ b/Source/WebCore/html/shadow/TextControlInnerElements.cpp @@ -285,7 +285,7 @@ void SpinButtonElement::defaultEventHandler(Event* event) MouseEvent* mouseEvent = static_cast(event); IntPoint local = roundedIntPoint(box->absoluteToLocal(mouseEvent->absoluteLocation(), false, true)); if (mouseEvent->type() == eventNames().mousedownEvent && mouseEvent->button() == LeftButton) { - if (box->borderBoxRect().contains(local)) { + if (box->pixelSnappedBorderBoxRect().contains(local)) { // The following functions of HTMLInputElement may run JavaScript // code which detaches this shadow node. We need to take a reference // and check renderer() after such function calls. @@ -304,7 +304,7 @@ void SpinButtonElement::defaultEventHandler(Event* event) } else if (mouseEvent->type() == eventNames().mouseupEvent && mouseEvent->button() == LeftButton) stopRepeatingTimer(); else if (event->type() == eventNames().mousemoveEvent) { - if (box->borderBoxRect().contains(local)) { + if (box->pixelSnappedBorderBoxRect().contains(local)) { if (!m_capturing) { if (Frame* frame = document()->frame()) { frame->eventHandler()->setCapturingMouseEventsNode(this); diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp index ef9ad33..cfb5b7d 100755 --- a/Source/WebCore/rendering/RenderBlock.cpp +++ b/Source/WebCore/rendering/RenderBlock.cpp @@ -1666,7 +1666,7 @@ void RenderBlock::addVisualOverflowFromTheme() if (!style()->hasAppearance()) return; - IntRect inflatedRect = borderBoxRect(); + IntRect inflatedRect = pixelSnappedBorderBoxRect(); theme()->adjustRepaintRect(this, inflatedRect); addVisualOverflow(inflatedRect); } diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h index 0a50753..761bcfe 100644 --- a/Source/WebCore/rendering/RenderBox.h +++ b/Source/WebCore/rendering/RenderBox.h @@ -133,8 +133,9 @@ public: IntRect pixelSnappedFrameRect() const { return pixelSnappedIntRect(m_frameRect); } void setFrameRect(const LayoutRect& rect) { m_frameRect = rect; } - IntRect borderBoxRect() const { return IntRect(IntPoint(), IntSize(m_frameRect.pixelSnappedWidth(), m_frameRect.pixelSnappedHeight())); } - virtual IntRect borderBoundingBox() const { return borderBoxRect(); } + LayoutRect borderBoxRect() const { return LayoutRect(LayoutPoint(), size()); } + IntRect pixelSnappedBorderBoxRect() const { return IntRect(IntPoint(), m_frameRect.pixelSnappedSize()); } + virtual IntRect borderBoundingBox() const { return pixelSnappedBorderBoxRect(); } // The content area of the box (excludes padding - and intrinsic padding for table cells, etc... - and border). LayoutRect contentBoxRect() const { return LayoutRect(borderLeft() + paddingLeft(), borderTop() + paddingTop(), contentWidth(), contentHeight()); } @@ -454,7 +455,7 @@ public: LayoutRect layoutOverflowRectForPropagation(RenderStyle*) const; RenderOverflow* hasRenderOverflow() const { return m_overflow.get(); } - bool hasVisualOverflow() const { return m_overflow && !borderBoxRect().contains(pixelSnappedIntRect(m_overflow->visualOverflowRect())); } + bool hasVisualOverflow() const { return m_overflow && !borderBoxRect().contains(m_overflow->visualOverflowRect()); } virtual bool needsPreferredWidthsRecalculation() const; virtual void computeIntrinsicRatioInformation(FloatSize& /* intrinsicSize */, double& /* intrinsicRatio */, bool& /* isPercentageIntrinsicSize */) const { } diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp index d2bc6bf..132881f 100644 --- a/Source/WebCore/rendering/RenderLayer.cpp +++ b/Source/WebCore/rendering/RenderLayer.cpp @@ -1994,7 +1994,7 @@ IntRect RenderLayer::scrollCornerRect() const bool hasVerticalBar = verticalScrollbar(); bool hasResizer = renderer()->style()->resize() != RESIZE_NONE; if ((hasHorizontalBar && hasVerticalBar) || (hasResizer && (hasHorizontalBar || hasVerticalBar))) - return cornerRect(this, renderBox()->borderBoxRect()); + return cornerRect(this, renderBox()->pixelSnappedBorderBoxRect()); return IntRect(); } @@ -2013,7 +2013,7 @@ IntRect RenderLayer::scrollCornerAndResizerRect() const return IntRect(); IntRect scrollCornerAndResizer = scrollCornerRect(); if (scrollCornerAndResizer.isEmpty()) - scrollCornerAndResizer = resizerCornerRect(this, pixelSnappedIntRect(box->borderBoxRect())); + scrollCornerAndResizer = resizerCornerRect(this, box->pixelSnappedBorderBoxRect()); return scrollCornerAndResizer; } @@ -2115,7 +2115,7 @@ LayoutUnit RenderLayer::horizontalScrollbarStart(int minX) const const RenderBox* box = renderBox(); int x = minX + box->borderLeft(); if (renderer()->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft()) - x += m_vBar ? m_vBar->width() : resizerCornerRect(this, box->borderBoxRect()).width(); + x += m_vBar ? m_vBar->width() : resizerCornerRect(this, box->pixelSnappedBorderBoxRect()).width(); return x; } @@ -2318,7 +2318,7 @@ void RenderLayer::positionOverflowControls(const IntSize& offsetFromLayer) if (!box) return; - const IntRect borderBox = box->borderBoxRect(); + const IntRect borderBox = box->pixelSnappedBorderBoxRect(); const IntRect& scrollCorner = scrollCornerRect(); IntRect absBounds(borderBox.location() + offsetFromLayer, borderBox.size()); if (m_vBar) @@ -2657,7 +2657,7 @@ void RenderLayer::paintResizer(GraphicsContext* context, const IntPoint& paintOf RenderBox* box = renderBox(); ASSERT(box); - IntRect absRect = resizerCornerRect(this, box->borderBoxRect()); + IntRect absRect = resizerCornerRect(this, box->pixelSnappedBorderBoxRect()); absRect.moveBy(paintOffset); if (!absRect.intersects(damageRect)) return; @@ -2712,7 +2712,7 @@ bool RenderLayer::hitTestOverflowControls(HitTestResult& result, const IntPoint& IntRect resizeControlRect; if (renderer()->style()->resize() != RESIZE_NONE) { - resizeControlRect = resizerCornerRect(this, box->borderBoxRect()); + resizeControlRect = resizerCornerRect(this, box->pixelSnappedBorderBoxRect()); if (resizeControlRect.contains(localPoint)) return true; } diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp index c98bafd..1b4b730 100644 --- a/Source/WebCore/rendering/RenderLayerBacking.cpp +++ b/Source/WebCore/rendering/RenderLayerBacking.cpp @@ -503,7 +503,7 @@ void RenderLayerBacking::updateGraphicsLayerGeometry() } if (m_owningLayer->hasTransform()) { - const IntRect borderBox = toRenderBox(renderer())->borderBoxRect(); + const IntRect borderBox = toRenderBox(renderer())->pixelSnappedBorderBoxRect(); // Get layout bounds in the coords of compAncestor to match relativeCompositingBounds. IntRect layerBounds = IntRect(delta, borderBox.size()); @@ -1296,7 +1296,7 @@ bool RenderLayerBacking::startAnimation(double timeOffset, const Animation* anim bool didAnimateFilter = false; #endif - if (hasTransform && m_graphicsLayer->addAnimation(transformVector, toRenderBox(renderer())->borderBoxRect().size(), anim, keyframes.animationName(), timeOffset)) { + if (hasTransform && m_graphicsLayer->addAnimation(transformVector, toRenderBox(renderer())->pixelSnappedBorderBoxRect().size(), anim, keyframes.animationName(), timeOffset)) { didAnimateTransform = true; compositor()->didStartAcceleratedAnimation(CSSPropertyWebkitTransform); } @@ -1361,7 +1361,7 @@ bool RenderLayerBacking::startTransition(double timeOffset, CSSPropertyID proper KeyframeValueList transformVector(AnimatedPropertyWebkitTransform); transformVector.insert(new TransformAnimationValue(0, &fromStyle->transform())); transformVector.insert(new TransformAnimationValue(1, &toStyle->transform())); - if (m_graphicsLayer->addAnimation(transformVector, toRenderBox(renderer())->borderBoxRect().size(), transformAnim, GraphicsLayer::animationNameForTransition(AnimatedPropertyWebkitTransform), timeOffset)) { + if (m_graphicsLayer->addAnimation(transformVector, toRenderBox(renderer())->pixelSnappedBorderBoxRect().size(), transformAnim, GraphicsLayer::animationNameForTransition(AnimatedPropertyWebkitTransform), timeOffset)) { // To ensure that the correct transform is visible when the animation ends, also set the final transform. updateLayerTransform(toStyle); didAnimateTransform = true; diff --git a/Source/WebCore/rendering/RenderTable.cpp b/Source/WebCore/rendering/RenderTable.cpp index a0b5667..5e89e2b 100644 --- a/Source/WebCore/rendering/RenderTable.cpp +++ b/Source/WebCore/rendering/RenderTable.cpp @@ -509,7 +509,7 @@ void RenderTable::addOverflowFromChildren() int bottomBorderOverflow = height() + outerBorderBottom() - borderBottom(); int topBorderOverflow = borderTop() - outerBorderTop(); IntRect borderOverflowRect(leftBorderOverflow, topBorderOverflow, rightBorderOverflow - leftBorderOverflow, bottomBorderOverflow - topBorderOverflow); - if (borderOverflowRect != borderBoxRect()) { + if (borderOverflowRect != pixelSnappedBorderBoxRect()) { addLayoutOverflow(borderOverflowRect); addVisualOverflow(borderOverflowRect); } diff --git a/Source/WebCore/rendering/RenderThemeMac.mm b/Source/WebCore/rendering/RenderThemeMac.mm index b71ff00..0075452 100644 --- a/Source/WebCore/rendering/RenderThemeMac.mm +++ b/Source/WebCore/rendering/RenderThemeMac.mm @@ -1534,7 +1534,7 @@ bool RenderThemeMac::paintSearchFieldCancelButton(RenderObject* o, const PaintIn float zoomLevel = o->style()->effectiveZoom(); - FloatRect localBounds = [search cancelButtonRectForBounds:NSRect(input->renderBox()->borderBoxRect())]; + FloatRect localBounds = [search cancelButtonRectForBounds:NSRect(input->renderBox()->pixelSnappedBorderBoxRect())]; #if ENABLE(INPUT_SPEECH) // Take care of cases where the cancel button was not aligned with the right border of the input element (for e.g. @@ -1619,7 +1619,7 @@ bool RenderThemeMac::paintSearchFieldResultsDecoration(RenderObject* o, const Pa updateActiveState([search searchButtonCell], o); - FloatRect localBounds = [search searchButtonRectForBounds:NSRect(input->renderBox()->borderBoxRect())]; + FloatRect localBounds = [search searchButtonRectForBounds:NSRect(input->renderBox()->pixelSnappedBorderBoxRect())]; localBounds = convertToPaintingRect(input->renderer(), o, localBounds, r); [[search searchButtonCell] drawWithFrame:localBounds inView:documentViewFor(o)]; @@ -1655,7 +1655,7 @@ bool RenderThemeMac::paintSearchFieldResultsButton(RenderObject* o, const PaintI GraphicsContextStateSaver stateSaver(*paintInfo.context); float zoomLevel = o->style()->effectiveZoom(); - FloatRect localBounds = [search searchButtonRectForBounds:NSRect(input->renderBox()->borderBoxRect())]; + FloatRect localBounds = [search searchButtonRectForBounds:NSRect(input->renderBox()->pixelSnappedBorderBoxRect())]; localBounds = convertToPaintingRect(input->renderer(), o, localBounds, r); IntRect unzoomedRect(localBounds); diff --git a/Source/WebCore/rendering/svg/RenderSVGRoot.cpp b/Source/WebCore/rendering/svg/RenderSVGRoot.cpp index d5d5954..2c15d9b 100644 --- a/Source/WebCore/rendering/svg/RenderSVGRoot.cpp +++ b/Source/WebCore/rendering/svg/RenderSVGRoot.cpp @@ -256,7 +256,7 @@ void RenderSVGRoot::layout() void RenderSVGRoot::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& adjustedPaintOffset) { // An empty viewport disables rendering. - if (borderBoxRect().isEmpty()) + if (pixelSnappedBorderBoxRect().isEmpty()) return; // Don't paint, if the context explicitely disabled it. @@ -368,7 +368,7 @@ void RenderSVGRoot::computeFloatRectForRepaint(RenderBoxModelObject* repaintCont repaintRect = m_localToBorderBoxTransform.mapRect(repaintRect); // Apply initial viewport clip - not affected by overflow settings - repaintRect.intersect(borderBoxRect()); + repaintRect.intersect(pixelSnappedBorderBoxRect()); const SVGRenderStyle* svgStyle = style()->svgStyle(); if (const ShadowData* shadow = svgStyle->shadow()) -- 2.7.4