Log Message
Subpixel rendering: Inline box decoration rounds to integral. https://bugs.webkit.org/show_bug.cgi?id=134523 <rdar://problem/17530298>
Reviewed by Darin Adler. This patch removes 2 integral roundings from InlineFlowBox: 1. Border and padding sizes are implicitly integral truncated by the 'int' return type of borderLogicalLeft/Right()/paddingLogicalLeft/Right(). It results in losing fractional border/padding values. 2. Painting rectangle is explicitly rounded which pushes border and other decoration elements to odd device pixel positions on retina displays. These values get pixel snapped right before calling in to GraphicsContext::*. Source/WebCore: Test: fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html * rendering/InlineBox.h: (WebCore::InlineBox::frameRect): * rendering/InlineFlowBox.cpp: (WebCore::InlineFlowBox::nodeAtPoint): (WebCore::InlineFlowBox::paintBoxDecorations): (WebCore::InlineFlowBox::paintMask): (WebCore::InlineFlowBox::roundedFrameRect): Deleted. * rendering/InlineFlowBox.h: (WebCore::InlineFlowBox::borderLogicalLeft): (WebCore::InlineFlowBox::borderLogicalRight): (WebCore::InlineFlowBox::paddingLogicalLeft): (WebCore::InlineFlowBox::paddingLogicalRight): LayoutTests: * fast/inline/hidpi-inline-text-decoration-with-subpixel-value-expected.html: Added. * fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html: Added. * platform/mac/css1/formatting_model/inline_elements-expected.txt:
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/platform/mac/css1/formatting_model/inline_elements-expected.txt
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/rendering/InlineBox.h
- trunk/Source/WebCore/rendering/InlineFlowBox.cpp
- trunk/Source/WebCore/rendering/InlineFlowBox.h
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (170874 => 170875)
--- trunk/LayoutTests/ChangeLog 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/LayoutTests/ChangeLog 2014-07-08 04:13:40 UTC (rev 170875)
@@ -1,3 +1,23 @@
+2014-07-07 Zalan Bujtas <za...@apple.com>
+
+ Subpixel rendering: Inline box decoration rounds to integral.
+ https://bugs.webkit.org/show_bug.cgi?id=134523
+ <rdar://problem/17530298>
+
+ Reviewed by Darin Adler.
+
+ This patch removes 2 integral roundings from InlineFlowBox:
+ 1. Border and padding sizes are implicitly integral truncated by the 'int' return type
+ of borderLogicalLeft/Right()/paddingLogicalLeft/Right(). It results in losing
+ fractional border/padding values.
+ 2. Painting rectangle is explicitly rounded which pushes border and
+ other decoration elements to odd device pixel positions on retina displays.
+ These values get pixel snapped right before calling in to GraphicsContext::*.
+
+ * fast/inline/hidpi-inline-text-decoration-with-subpixel-value-expected.html: Added.
+ * fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html: Added.
+ * platform/mac/css1/formatting_model/inline_elements-expected.txt:
+
2014-07-07 Hunseop Jeong <hs85.je...@samsung.com>
[EFL] gardening after r170864
https://bugs.webkit.org/show_bug.cgi?id=134713
Added: trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value-expected.html (0 => 170875)
--- trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value-expected.html (rev 0)
+++ trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value-expected.html 2014-07-08 04:13:40 UTC (rev 170875)
@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that inline border is positioned properly.</title>
+<style>
+ div {
+ border: 0.5px solid blue;
+ position: absolute;
+ width: 26px;
+ height: 10px;
+ }
+</style>
+</head>
+<body>
+
+<script>
+ for(i = 0; i < 9; ++i) {
+ for(j = 1; j < 3; ++j) {
+ var e = document.createElement("div");
+ e.style.left = j * 30 + "px";
+ e.style.top = 36.5 + (30 * i) + "px";
+ document.body.appendChild(e);
+ }
+ }
+
+ for(i = 0; i < 9; ++i) {
+ var e = document.createElement("div");
+ e.style.width = "26.5px";
+ e.style.height = "10px";
+ e.style.left = 3 * 30 + "px";
+ e.style.top = 36.5 + (30 * i) + "px";
+ document.body.appendChild(e);
+ }
+
+ for(i = 0; i < 9; ++i) {
+ var e = document.createElement("div");
+ e.style.width = "26.5px";
+ e.style.height = "11px";
+ e.style.left = 4 * 30 + "px";
+ e.style.top = 36 + (30 * i) + "px";
+ document.body.appendChild(e);
+ }
+
+ for(i = 0; i < 9; ++i) {
+ var e = document.createElement("div");
+ e.style.width = "27px";
+ e.style.height = "11px";
+ e.style.left = 5 * 30 + "px";
+ e.style.top = 36 + (30 * i) + "px";
+ document.body.appendChild(e);
+ }
+
+ for(i = 0; i < 9; ++i) {
+ for(j = 6; j < 8; ++j) {
+ var e = document.createElement("div");
+ e.style.left = j * 30 + "px";
+ e.style.top = 36 + (30 * i) + "px";
+ e.style.borderWidth = "1px";
+ document.body.appendChild(e);
+ }
+ }
+
+ for(i = 0; i < 9; ++i) {
+ var e = document.createElement("div");
+ e.style.width = "26.5px";
+ e.style.height = "10px";
+ e.style.left = 8 * 30 + "px";
+ e.style.top = 36 + (30 * i) + "px";
+ e.style.borderWidth = "1px";
+ document.body.appendChild(e);
+ }
+
+ for(i = 0; i < 9; ++i) {
+ var e = document.createElement("div");
+ e.style.width = "26.5px";
+ e.style.height = "11px";
+ e.style.left = 9 * 30 + "px";
+ e.style.top = 35.5 + (30 * i) + "px";
+ e.style.borderWidth = "1px";
+ document.body.appendChild(e);
+ }
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html (0 => 170875)
--- trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html (rev 0)
+++ trunk/LayoutTests/fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html 2014-07-08 04:13:40 UTC (rev 170875)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that inline border is positioned properly.</title>
+<style>
+ div {
+ position: absolute;
+ }
+
+ span {
+ font-family: 'Ahem';
+ font-size: 8px;
+ border: 0.5px solid blue;
+ color: white;
+ padding: 1px;
+ }
+</style>
+</head>
+<body>
+<p id="container"></p>
+<script>
+ var container = document.getElementById("container");
+ for (i = 1; i < 10; ++i) {
+ borderWidth = 0.5;
+ for (j = 1; j < 10; ++j) {
+ var e = document.createElement("div");
+ e.style.top = (30 * i) + "px";
+ e.style.left = (30 * j) + "px";
+
+ var s = document.createElement("span");
+ s.style.borderWidth = borderWidth + "px";
+ s.innerHTML = "foo";
+
+ e.appendChild(s);
+ container.appendChild(e);
+ borderWidth += 0.1;
+ }
+ }
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/mac/css1/formatting_model/inline_elements-expected.txt (170874 => 170875)
--- trunk/LayoutTests/platform/mac/css1/formatting_model/inline_elements-expected.txt 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/LayoutTests/platform/mac/css1/formatting_model/inline_elements-expected.txt 2014-07-08 04:13:40 UTC (rev 170875)
@@ -25,9 +25,9 @@
RenderBlock {P} at (0,170) size 769x192
RenderText {#text} at (0,7) size 187x18
text run at (0,7) width 187: "This is a paragraph that has a "
- RenderInline {SPAN} at (0,0) size 761x172 [border: (10px solid #FF0000)]
- RenderText {#text} at (239,7) size 761x146
- text run at (239,7) width 522: "very long span in it, and the span has a 10px red border separated from the span by"
+ RenderInline {SPAN} at (0,0) size 762x172 [border: (10px solid #FF0000)]
+ RenderText {#text} at (239,7) size 762x146
+ text run at (239,7) width 523: "very long span in it, and the span has a 10px red border separated from the span by"
text run at (0,39) width 167: "2pt, and a margin of 30pt. "
text run at (167,39) width 560: "The padding and border should be present on all sides of the span (although vertical lines"
text run at (0,71) width 539: "should appear only at the beginning and the end of the whole span, not on each line). "
@@ -35,23 +35,23 @@
text run at (0,103) width 388: "should all be noticeable at the beginning and end of the span. "
text run at (388,103) width 366: "However, the line height should not be changed by any of"
text run at (0,135) width 585: "them, so the margin should be unnoticeable and the border should overlap text on other lines."
- RenderText {#text} at (637,135) size 761x50
- text run at (637,135) width 4: " "
- text run at (641,135) width 120: "The line spacing in"
+ RenderText {#text} at (637,135) size 762x50
+ text run at (637,135) width 5: " "
+ text run at (641,135) width 121: "The line spacing in"
text run at (0,167) width 336: "the whole paragraph should be 200% of the font size."
RenderBlock {P} at (0,378) size 769x64
RenderText {#text} at (0,0) size 159x15
text run at (0,0) width 159: "This is a paragraph that has a "
RenderInline {SPAN} at (0,0) size 764x93 [border: (12px solid #FF0000)]
RenderText {#text} at (173,0) size 764x63
- text run at (173,0) width 552: "very long span in it, and the span has a 12px red border separated from the span by 2pt of padding (the"
+ text run at (173,0) width 553: "very long span in it, and the span has a 12px red border separated from the span by 2pt of padding (the"
text run at (0,16) width 764: "difference between the line-height and the font-size), which should overlap with the lines of text above and below the span, since the padding"
text run at (0,32) width 240: "and border should not effect the line height. "
text run at (240,32) width 524: "The span's border should have vertical lines only at the beginning and end of the whole span, not"
text run at (0,48) width 69: "on each line."
- RenderText {#text} at (83,48) size 415x15
- text run at (83,48) width 3: " "
- text run at (86,48) width 412: "The line spacing in the whole paragraph should be 12pt, with font-size 10pt."
+ RenderText {#text} at (83,48) size 416x15
+ text run at (83,48) width 4: " "
+ text run at (86,48) width 413: "The line spacing in the whole paragraph should be 12pt, with font-size 10pt."
RenderTable {TABLE} at (0,455) size 769x309 [border: (1px outset #808080)]
RenderTableSection {TBODY} at (1,1) size 767x306
RenderTableRow {TR} at (0,0) size 767x26
@@ -69,7 +69,7 @@
text run at (0,7) width 187: "This is a paragraph that has a "
RenderInline {SPAN} at (0,0) size 747x172 [border: (10px solid #FF0000)]
RenderText {#text} at (239,7) size 747x146
- text run at (239,7) width 502: "very long span in it, and the span has a 10px red border separated from the span"
+ text run at (239,7) width 503: "very long span in it, and the span has a 10px red border separated from the span"
text run at (0,39) width 187: "by 2pt, and a margin of 30pt. "
text run at (187,39) width 560: "The padding and border should be present on all sides of the span (although vertical lines"
text run at (0,71) width 539: "should appear only at the beginning and the end of the whole span, not on each line). "
@@ -77,20 +77,20 @@
text run at (0,103) width 388: "should all be noticeable at the beginning and end of the span. "
text run at (388,103) width 349: "However, the line height should not be changed by any"
text run at (0,135) width 602: "of them, so the margin should be unnoticeable and the border should overlap text on other lines."
- RenderText {#text} at (654,135) size 710x50
- text run at (654,135) width 4: " "
- text run at (658,135) width 52: "The line"
+ RenderText {#text} at (654,135) size 711x50
+ text run at (654,135) width 5: " "
+ text run at (658,135) width 53: "The line"
text run at (0,167) width 404: "spacing in the whole paragraph should be 200% of the font size."
RenderBlock {P} at (4,212) size 747x64
RenderText {#text} at (0,0) size 159x15
text run at (0,0) width 159: "This is a paragraph that has a "
- RenderInline {SPAN} at (0,0) size 725x93 [border: (12px solid #FF0000)]
- RenderText {#text} at (173,0) size 725x63
- text run at (173,0) width 552: "very long span in it, and the span has a 12px red border separated from the span by 2pt of padding (the"
+ RenderInline {SPAN} at (0,0) size 726x93 [border: (12px solid #FF0000)]
+ RenderText {#text} at (173,0) size 726x63
+ text run at (173,0) width 553: "very long span in it, and the span has a 12px red border separated from the span by 2pt of padding (the"
text run at (0,16) width 716: "difference between the line-height and the font-size), which should overlap with the lines of text above and below the span, since the"
text run at (0,32) width 288: "padding and border should not effect the line height. "
text run at (288,32) width 436: "The span's border should have vertical lines only at the beginning and end of the"
text run at (0,48) width 157: "whole span, not on each line."
- RenderText {#text} at (171,48) size 415x15
- text run at (171,48) width 3: " "
- text run at (174,48) width 412: "The line spacing in the whole paragraph should be 12pt, with font-size 10pt."
+ RenderText {#text} at (171,48) size 416x15
+ text run at (171,48) width 4: " "
+ text run at (174,48) width 413: "The line spacing in the whole paragraph should be 12pt, with font-size 10pt."
Modified: trunk/Source/WebCore/ChangeLog (170874 => 170875)
--- trunk/Source/WebCore/ChangeLog 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/Source/WebCore/ChangeLog 2014-07-08 04:13:40 UTC (rev 170875)
@@ -1,5 +1,36 @@
2014-07-07 Zalan Bujtas <za...@apple.com>
+ Subpixel rendering: Inline box decoration rounds to integral.
+ https://bugs.webkit.org/show_bug.cgi?id=134523
+ <rdar://problem/17530298>
+
+ Reviewed by Darin Adler.
+
+ This patch removes 2 integral roundings from InlineFlowBox:
+ 1. Border and padding sizes are implicitly integral truncated by the 'int' return type
+ of borderLogicalLeft/Right()/paddingLogicalLeft/Right(). It results in losing
+ fractional border/padding values.
+ 2. Painting rectangle is explicitly rounded which pushes border and
+ other decoration elements to odd device pixel positions on retina displays.
+ These values get pixel snapped right before calling in to GraphicsContext::*.
+
+ Test: fast/inline/hidpi-inline-text-decoration-with-subpixel-value.html
+
+ * rendering/InlineBox.h:
+ (WebCore::InlineBox::frameRect):
+ * rendering/InlineFlowBox.cpp:
+ (WebCore::InlineFlowBox::nodeAtPoint):
+ (WebCore::InlineFlowBox::paintBoxDecorations):
+ (WebCore::InlineFlowBox::paintMask):
+ (WebCore::InlineFlowBox::roundedFrameRect): Deleted.
+ * rendering/InlineFlowBox.h:
+ (WebCore::InlineFlowBox::borderLogicalLeft):
+ (WebCore::InlineFlowBox::borderLogicalRight):
+ (WebCore::InlineFlowBox::paddingLogicalLeft):
+ (WebCore::InlineFlowBox::paddingLogicalRight):
+
+2014-07-07 Zalan Bujtas <za...@apple.com>
+
Pass RenderLayer reference instead of pointer to RenderLayer::paintingExtent().
https://bugs.webkit.org/show_bug.cgi?id=134714
Modified: trunk/Source/WebCore/rendering/InlineBox.h (170874 => 170875)
--- trunk/Source/WebCore/rendering/InlineBox.h 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/Source/WebCore/rendering/InlineBox.h 2014-07-08 04:13:40 UTC (rev 170875)
@@ -210,6 +210,7 @@
float logicalHeight() const;
FloatRect logicalFrameRect() const { return isHorizontal() ? FloatRect(m_topLeft.x(), m_topLeft.y(), m_logicalWidth, logicalHeight()) : FloatRect(m_topLeft.y(), m_topLeft.x(), m_logicalWidth, logicalHeight()); }
+ FloatRect frameRect() const { return FloatRect(topLeft(), size()); }
virtual int baselinePosition(FontBaseline baselineType) const;
virtual LayoutUnit lineHeight() const;
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.cpp (170874 => 170875)
--- trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2014-07-08 04:13:40 UTC (rev 170875)
@@ -78,18 +78,6 @@
return totWidth;
}
-IntRect InlineFlowBox::roundedFrameRect() const
-{
- // Begin by snapping the x and y coordinates to the nearest pixel.
- int snappedX = lroundf(x());
- int snappedY = lroundf(y());
-
- int snappedMaxX = lroundf(x() + width());
- int snappedMaxY = lroundf(y() + height());
-
- return IntRect(snappedX, snappedY, snappedMaxX - snappedX, snappedMaxY - snappedY);
-}
-
static void setHasTextDescendantsOnAncestors(InlineFlowBox* box)
{
while (box && !box->hasTextDescendants()) {
@@ -1061,7 +1049,7 @@
// Do not hittest content beyond the ellipsis box.
if (isRootInlineBox() && hasEllipsisBox()) {
const EllipsisBox* ellipsisBox = root().ellipsisBox();
- LayoutRect boundsRect(roundedFrameRect());
+ FloatRect boundsRect(frameRect());
if (isHorizontal())
renderer().style().isLeftToRightDirection() ? boundsRect.shiftXEdgeTo(ellipsisBox->right()) : boundsRect.setWidth(ellipsisBox->left() - left());
@@ -1075,25 +1063,19 @@
return false;
}
- LayoutRect frameRect = roundedFrameRect();
- LayoutUnit minX = frameRect.x();
- LayoutUnit minY = frameRect.y();
- LayoutUnit width = frameRect.width();
- LayoutUnit height = frameRect.height();
-
// Constrain our hit testing to the line top and bottom if necessary.
bool noQuirksMode = renderer().document().inNoQuirksMode();
if (!noQuirksMode && !hasTextChildren() && !(descendantsHaveSameLineHeightAndBaseline() && hasTextDescendants())) {
RootInlineBox& rootBox = root();
- LayoutUnit& top = isHorizontal() ? minY : minX;
- LayoutUnit& logicalHeight = isHorizontal() ? height : width;
+ LayoutUnit top = isHorizontal() ? y() : x();
+ LayoutUnit logicalHeight = isHorizontal() ? height() : width();
LayoutUnit bottom = std::min(rootBox.lineBottom(), top + logicalHeight);
top = std::max(rootBox.lineTop(), top);
logicalHeight = bottom - top;
}
// Move x/y to our coordinates.
- LayoutRect rect(minX, minY, width, height);
+ FloatRect rect(frameRect());
flipForWritingMode(rect);
rect.moveBy(accumulatedOffset);
@@ -1302,9 +1284,7 @@
if (!paintInfo.shouldPaintWithinRoot(renderer()) || renderer().style().visibility() != VISIBLE || paintInfo.phase != PaintPhaseForeground)
return;
- // Pixel snap background/border painting.
- LayoutRect frameRect = roundedFrameRect();
-
+ LayoutRect frameRect(this->frameRect());
constrainToLineTopAndBottomIfNeeded(frameRect);
// Move x/y to our coordinates.
@@ -1376,9 +1356,7 @@
if (!paintInfo.shouldPaintWithinRoot(renderer()) || renderer().style().visibility() != VISIBLE || paintInfo.phase != PaintPhaseMask)
return;
- // Pixel snap mask painting.
- LayoutRect frameRect = roundedFrameRect();
-
+ LayoutRect frameRect(this->frameRect());
constrainToLineTopAndBottomIfNeeded(frameRect);
// Move x/y to our coordinates.
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.h (170874 => 170875)
--- trunk/Source/WebCore/rendering/InlineFlowBox.h 2014-07-08 04:00:27 UTC (rev 170874)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.h 2014-07-08 04:13:40 UTC (rev 170875)
@@ -113,8 +113,6 @@
virtual void clearTruncation() override;
- IntRect roundedFrameRect() const;
-
void paintBoxDecorations(PaintInfo&, const LayoutPoint&);
void paintMask(PaintInfo&, const LayoutPoint&);
void paintFillLayers(const PaintInfo&, const Color&, const FillLayer*, const LayoutRect&, CompositeOperator = CompositeSourceOver);
@@ -140,25 +138,25 @@
return 0;
return isHorizontal() ? renderer().marginRight() : renderer().marginBottom();
}
- int borderLogicalLeft() const
+ float borderLogicalLeft() const
{
if (!includeLogicalLeftEdge())
return 0;
return isHorizontal() ? lineStyle().borderLeftWidth() : lineStyle().borderTopWidth();
}
- int borderLogicalRight() const
+ float borderLogicalRight() const
{
if (!includeLogicalRightEdge())
return 0;
return isHorizontal() ? lineStyle().borderRightWidth() : lineStyle().borderBottomWidth();
}
- int paddingLogicalLeft() const
+ float paddingLogicalLeft() const
{
if (!includeLogicalLeftEdge())
return 0;
return isHorizontal() ? renderer().paddingLeft() : renderer().paddingTop();
}
- int paddingLogicalRight() const
+ float paddingLogicalRight() const
{
if (!includeLogicalRightEdge())
return 0;
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
