Simple line layout: Use float types wherever possible to match line tree.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2015 17:23:26 +0000 (17:23 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2015 17:23:26 +0000 (17:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148444

Reviewed by Antti Koivisto.

To match inline tree output, we should try to match the data types as far as precision goes.

This patch also fixes the confusing mismatch between Run::baseline().x() and Run::rect().x().
They are both supposed to return the left edge of the run. However Run::rect().x() returns a rounded
LayoutUnit of the logical left, while Run::baseline().x() returns the correct logical left.
With this patch
  1. baseline position does not include logical left anymore.
  2. Run::rect().x() does not round the logical left coordinate anymore.

* rendering/RenderTreeAsText.cpp:
(WebCore::writeSimpleLine):
* rendering/SimpleLineLayoutFunctions.cpp:
(WebCore::SimpleLineLayout::paintFlow):
(WebCore::SimpleLineLayout::collectFlowOverflow):
(WebCore::SimpleLineLayout::collectAbsoluteRects):
(WebCore::SimpleLineLayout::showLineLayoutForFlow):
* rendering/SimpleLineLayoutResolver.cpp:
(WebCore::SimpleLineLayout::linePosition):
(WebCore::SimpleLineLayout::lineSize):
(WebCore::SimpleLineLayout::RunResolver::Run::rect):
(WebCore::SimpleLineLayout::LineResolver::Iterator::operator*):
(WebCore::SimpleLineLayout::baselinePosition): Deleted.
(WebCore::SimpleLineLayout::RunResolver::Run::baseline): Deleted.
* rendering/SimpleLineLayoutResolver.h:
(WebCore::SimpleLineLayout::RunResolver::Run::baselinePosition):
(WebCore::SimpleLineLayout::RunResolver::Run::computeBaselinePosition):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTreeAsText.cpp
Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp
Source/WebCore/rendering/SimpleLineLayoutResolver.cpp
Source/WebCore/rendering/SimpleLineLayoutResolver.h

index 1992165..76d7981 100644 (file)
@@ -1,5 +1,39 @@
 2015-08-27  Zalan Bujtas  <zalan@apple.com>
 
+        Simple line layout: Use float types wherever possible to match line tree.
+        https://bugs.webkit.org/show_bug.cgi?id=148444
+
+        Reviewed by Antti Koivisto.
+
+        To match inline tree output, we should try to match the data types as far as precision goes.
+
+        This patch also fixes the confusing mismatch between Run::baseline().x() and Run::rect().x(). 
+        They are both supposed to return the left edge of the run. However Run::rect().x() returns a rounded
+        LayoutUnit of the logical left, while Run::baseline().x() returns the correct logical left.          
+        With this patch 
+          1. baseline position does not include logical left anymore.
+          2. Run::rect().x() does not round the logical left coordinate anymore.
+
+        * rendering/RenderTreeAsText.cpp:
+        (WebCore::writeSimpleLine):
+        * rendering/SimpleLineLayoutFunctions.cpp:
+        (WebCore::SimpleLineLayout::paintFlow):
+        (WebCore::SimpleLineLayout::collectFlowOverflow):
+        (WebCore::SimpleLineLayout::collectAbsoluteRects):
+        (WebCore::SimpleLineLayout::showLineLayoutForFlow):
+        * rendering/SimpleLineLayoutResolver.cpp:
+        (WebCore::SimpleLineLayout::linePosition):
+        (WebCore::SimpleLineLayout::lineSize):
+        (WebCore::SimpleLineLayout::RunResolver::Run::rect):
+        (WebCore::SimpleLineLayout::LineResolver::Iterator::operator*):
+        (WebCore::SimpleLineLayout::baselinePosition): Deleted.
+        (WebCore::SimpleLineLayout::RunResolver::Run::baseline): Deleted.
+        * rendering/SimpleLineLayoutResolver.h:
+        (WebCore::SimpleLineLayout::RunResolver::Run::baselinePosition):
+        (WebCore::SimpleLineLayout::RunResolver::Run::computeBaselinePosition):
+
+2015-08-27  Zalan Bujtas  <zalan@apple.com>
+
         Subpixel positioned iframe's repaint area calculation problem.
         https://bugs.webkit.org/show_bug.cgi?id=148422
 
index c6c420a..9528903 100644 (file)
@@ -484,7 +484,7 @@ static void writeTextRun(TextStream& ts, const RenderText& o, const InlineTextBo
     ts << "\n";
 }
 
-static void writeSimpleLine(TextStream& ts, const RenderText& o, const LayoutRect& rect, StringView text)
+static void writeSimpleLine(TextStream& ts, const RenderText& o, const FloatRect& rect, StringView text)
 {
     int x = rect.x();
     int y = rect.y();
index 9ef5eb0..ddb0adb 100644 (file)
@@ -80,6 +80,7 @@ void paintFlow(const RenderBlockFlow& flow, const Layout& layout, PaintInfo& pai
 
     auto resolver = runResolver(flow, layout);
     float strokeOverflow = ceilf(flow.style().textStrokeWidth());
+    float deviceScaleFactor = flow.document().deviceScaleFactor();
     for (const auto& run : resolver.rangeForRect(paintRect)) {
         FloatRect rect = run.rect();
         rect.inflate(strokeOverflow);
@@ -88,11 +89,10 @@ void paintFlow(const RenderBlockFlow& flow, const Layout& layout, PaintInfo& pai
         TextRun textRun(run.text());
         textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
         textRun.setXPos(run.rect().x());
-        FloatPoint textOrigin = run.baseline() + paintOffset;
-        textOrigin.setY(roundToDevicePixel(LayoutUnit(textOrigin.y()), flow.document().deviceScaleFactor()));
+        FloatPoint textOrigin = FloatPoint(rect.x() + paintOffset.x(), roundToDevicePixel(run.baselinePosition() + paintOffset.y(), deviceScaleFactor));
         context.drawText(font, textRun, textOrigin);
         if (debugBordersEnabled)
-            paintDebugBorders(context, run.rect(), paintOffset);
+            paintDebugBorders(context, LayoutRect(run.rect()), paintOffset);
     }
 }
 
@@ -132,7 +132,7 @@ void collectFlowOverflow(RenderBlockFlow& flow, const Layout& layout)
     auto resolver = lineResolver(flow, layout);
     float strokeOverflow = ceilf(flow.style().textStrokeWidth());
     for (auto it = resolver.begin(), end = resolver.end(); it != end; ++it) {
-        auto rect = *it;
+        auto rect = LayoutRect(*it);
         rect.inflate(strokeOverflow);
         flow.addLayoutOverflow(rect);
         flow.addVisualOverflow(rect);
@@ -169,7 +169,7 @@ Vector<IntRect> collectAbsoluteRects(const RenderObject& renderer, const Layout&
     Vector<IntRect> rects;
     auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
     for (const auto& run : resolver.rangeForRenderer(renderer)) {
-        LayoutRect rect = run.rect();
+        FloatRect rect = run.rect();
         rects.append(enclosingIntRect(FloatRect(accumulatedOffset + rect.location(), rect.size())));
     }
     return rects;
@@ -204,15 +204,14 @@ void showLineLayoutForFlow(const RenderBlockFlow& flow, const Layout& layout, in
     auto resolver = runResolver(flow, layout);
     for (auto it = resolver.begin(), end = resolver.end(); it != end; ++it) {
         const auto& run = *it;
-        LayoutRect r = run.rect();
+        FloatRect rect = run.rect();
         printPrefix(printedCharacters, depth);
         if (run.start() < run.end()) {
             fprintf(stderr, "line %u run(%u, %u) (%.2f, %.2f) (%.2f, %.2f) \"%s\"\n", run.lineIndex(), run.start(), run.end(),
-                r.x().toFloat(), r.y().toFloat(), r.width().toFloat(), r.height().toFloat(), run.text().toStringWithoutCopying().utf8().data());
+                rect.x(), rect.y(), rect.width(), rect.height(), run.text().toStringWithoutCopying().utf8().data());
         } else {
             ASSERT(run.start() == run.end());
-            fprintf(stderr, "line break %u run(%u, %u) (%.2f, %.2f) (%.2f, %.2f)\n", run.lineIndex(), run.start(), run.end(),
-                r.x().toFloat(), r.y().toFloat(), r.width().toFloat(), r.height().toFloat());
+            fprintf(stderr, "line break %u run(%u, %u) (%.2f, %.2f) (%.2f, %.2f)\n", run.lineIndex(), run.start(), run.end(), rect.x(), rect.y(), rect.width(), rect.height());
         }
     }
 }
index de36398..4359fb8 100644 (file)
 namespace WebCore {
 namespace SimpleLineLayout {
 
-static float baselinePosition(float lineHeight, float baseline, int lineIndex)
+static FloatPoint linePosition(float logicalLeft, float logicalTop)
 {
-    return lineHeight * lineIndex + baseline;
+    return FloatPoint(logicalLeft, roundf(logicalTop));
 }
 
-static LayoutPoint linePosition(float logicalLeft, float logicalTop)
+static FloatSize lineSize(float logicalLeft, float logicalRight, float height)
 {
-    return LayoutPoint(LayoutUnit::fromFloatFloor(logicalLeft), roundToInt(logicalTop));
-}
-
-static LayoutSize lineSize(float logicalLeft, float logicalRight, float height)
-{
-    return LayoutSize(LayoutUnit::fromFloatCeil(logicalRight) - LayoutUnit::fromFloatFloor(logicalLeft), height);
+    return FloatSize(logicalRight - logicalLeft, height);
 }
 
 RunResolver::Run::Run(const Iterator& iterator)
@@ -53,13 +48,13 @@ RunResolver::Run::Run(const Iterator& iterator)
 {
 }
 
-LayoutRect RunResolver::Run::rect() const
+FloatRect RunResolver::Run::rect() const
 {
     auto& run = m_iterator.simpleRun();
     auto& resolver = m_iterator.resolver();
-    float baseline = baselinePosition(resolver.m_lineHeight, resolver.m_baseline, m_iterator.lineIndex());
-    LayoutPoint position = linePosition(run.logicalLeft, baseline - resolver.m_ascent + resolver.m_borderAndPaddingBefore);
-    LayoutSize size = lineSize(run.logicalLeft, run.logicalRight, resolver.m_ascent + resolver.m_descent);
+    float baseline = computeBaselinePosition();
+    FloatPoint position = linePosition(run.logicalLeft, baseline - resolver.m_ascent);
+    FloatSize size = lineSize(run.logicalLeft, run.logicalRight, resolver.m_ascent + resolver.m_descent);
     bool moveLineBreakToBaseline = false;
     if (run.start == run.end && m_iterator != resolver.begin() && m_iterator.inQuirksMode()) {
         auto previousRun = m_iterator;
@@ -67,17 +62,8 @@ LayoutRect RunResolver::Run::rect() const
         moveLineBreakToBaseline = !previousRun.simpleRun().isEndOfLine;
     }
     if (moveLineBreakToBaseline)
-        return LayoutRect(LayoutPoint(position.x(), baseline + resolver.m_borderAndPaddingBefore), LayoutSize(size.width(), std::max<float>(0, resolver.m_ascent - resolver.m_baseline.toFloat())));
-    return LayoutRect(position, size);
-}
-
-FloatPoint RunResolver::Run::baseline() const
-{
-    auto& resolver = m_iterator.resolver();
-    auto& run = m_iterator.simpleRun();
-
-    float baseline = baselinePosition(resolver.m_lineHeight, resolver.m_baseline, m_iterator.lineIndex());
-    return FloatPoint(run.logicalLeft, roundToInt(baseline + resolver.m_borderAndPaddingBefore));
+        return FloatRect(FloatPoint(position.x(), baseline), FloatSize(size.width(), std::max<float>(0, resolver.m_ascent - resolver.m_baseline.toFloat())));
+    return FloatRect(position, size);
 }
 
 StringView RunResolver::Run::text() const
@@ -204,14 +190,13 @@ LineResolver::Iterator::Iterator(RunResolver::Iterator runIterator)
 {
 }
 
-const LayoutRect LineResolver::Iterator::operator*() const
+const FloatRect LineResolver::Iterator::operator*() const
 {
     unsigned currentLine = m_runIterator.lineIndex();
     auto it = m_runIterator;
-    LayoutRect rect = (*it).rect();
+    FloatRect rect = (*it).rect();
     while (it.advance().lineIndex() == currentLine)
         rect.unite((*it).rect());
-
     return rect;
 }
 
index 467217f..1da4599 100644 (file)
@@ -61,14 +61,16 @@ public:
         unsigned start() const;
         unsigned end() const;
 
-        LayoutRect rect() const;
-        FloatPoint baseline() const;
+        FloatRect rect() const;
+        int baselinePosition() const;
         StringView text() const;
         bool isEndOfLine() const;
 
         unsigned lineIndex() const;
 
     private:
+        float computeBaselinePosition() const;
+
         const Iterator& m_iterator;
     };
 
@@ -136,7 +138,7 @@ public:
         bool operator==(const Iterator&) const;
         bool operator!=(const Iterator&) const;
 
-        const LayoutRect operator*() const;
+        const FloatRect operator*() const;
 
     private:
         RunResolver::Iterator m_runIterator;
@@ -167,6 +169,11 @@ inline unsigned RunResolver::Run::end() const
     return m_iterator.simpleRun().end;
 }
 
+inline int RunResolver::Run::baselinePosition() const
+{
+    return roundToInt(computeBaselinePosition());
+}
+
 inline bool RunResolver::Run::isEndOfLine() const
 {
     return m_iterator.simpleRun().isEndOfLine;
@@ -182,6 +189,12 @@ inline RunResolver::Iterator& RunResolver::Iterator::operator++()
     return advance();
 }
 
+inline float RunResolver::Run::computeBaselinePosition() const
+{
+    auto& resolver = m_iterator.resolver();
+    return resolver.m_lineHeight * lineIndex() + resolver.m_baseline + resolver.m_borderAndPaddingBefore;
+}
+
 inline RunResolver::Iterator& RunResolver::Iterator::operator--()
 {
     --m_runIndex;