Diff
Modified: trunk/LayoutTests/ChangeLog (172007 => 172008)
--- trunk/LayoutTests/ChangeLog 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/LayoutTests/ChangeLog 2014-08-04 22:01:04 UTC (rev 172008)
@@ -1,3 +1,25 @@
+2014-08-04 Zalan Bujtas <[email protected]>
+
+ Subpixel rendering: InlineTextBox mistakenly rounds offset value before painting.
+ https://bugs.webkit.org/show_bug.cgi?id=135470
+
+ Reviewed by Simon Fraser.
+
+ This patch removes the premature paint offset adjustment for inlines. Premature snapping
+ could alter the final painting coordinates and push content to wrong positions.
+
+ This patch also enforces WebCore's pixel snapping strategy (round) on text painting.
+ It ensures that text positioning is in sync with other painting related operations including
+ clipping, box decorations etc. Underlying graphics libraries can take different directions on
+ text snapping, for example CG ceils text coordinates vertically (in horizontal context,
+ with the current settings). It can lead to undesired side effects.
+
+ * fast/inline/hidpi-inline-selection-leaves-gap-expected.html: Added.
+ * fast/inline/hidpi-inline-selection-leaves-gap.html: Added.
+ * fast/multicol/newmulticol/multicol-clip-rounded-corners-expected.html:
+ * fast/multicol/newmulticol/multicol-clip-rounded-corners.html: pixels are distributed properly.
+ No need to have the special 122px shortened width for col2.
+
2014-08-04 Chris Fleizach <[email protected]>
AX: isWordEndMatch should allow for multiple word selections
Added: trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap-expected.html (0 => 172008)
--- trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap-expected.html (rev 0)
+++ trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap-expected.html 2014-08-04 22:01:04 UTC (rev 172008)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that inline text selection on subpixel position does not leave gaps at the inline boundary.</title>
+<style>
+ body {
+ margin: 0px;
+ font-family: "Ahem";
+ color: rgba(255, 255, 255, 0);
+ }
+
+ .bckg {
+ background: rgba(255, 0, 0, 0.9);
+ font-size: 16px;
+ }
+</style>
+</head>
+<body>
+<div id=container>
+ <div style="position: absolute; height: 16px; width: 0.5px; background-color: white;"></div>
+ <div class=bckg style="text-indent: 0.4px; margin-left: 0.1px;">foo</div>
+ <div class=bckg> </div>
+</div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap.html (0 => 172008)
--- trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap.html (rev 0)
+++ trunk/LayoutTests/fast/inline/hidpi-inline-selection-leaves-gap.html 2014-08-04 22:01:04 UTC (rev 172008)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that inline text selection on subpixel position does not leave gaps at the inline boundary.</title>
+<style>
+ ::selection {
+ background: rgba(255, 0, 0, 0.9);
+ }
+
+ body {
+ margin: 0px;
+ font-family: "Ahem";
+ color: rgba(255, 255, 255, 0);
+ }
+
+ div {
+ font-size: 16px;
+ }
+</style>
+</head>
+<body>
+<div id=container>
+ <div style="text-indent: 0.4px; margin-left: 0.1px;">foo</div>
+ <div> </div><br>
+</div>
+<script>
+ var range = document.createRange();
+ range.selectNode(document.getElementById("container"));
+ window.getSelection().addRange(range);
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners-expected.html (172007 => 172008)
--- trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners-expected.html 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners-expected.html 2014-08-04 22:01:04 UTC (rev 172008)
@@ -3,7 +3,7 @@
<head>
<style>
#multicol {
- width: 400px;
+ width: 401px;
height: 200px;
border: 5px solid black;
border-radius: 10%;
@@ -22,7 +22,6 @@
}
#col2 {
- width: 122px;
margin-left: 16px;
}
Modified: trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners.html (172007 => 172008)
--- trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners.html 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/LayoutTests/fast/multicol/newmulticol/multicol-clip-rounded-corners.html 2014-08-04 22:01:04 UTC (rev 172008)
@@ -4,7 +4,7 @@
<style>
#multicol {
-webkit-column-width: 100px;
- width: 400px;
+ width: 401px;
height: 200px;
border: 5px solid black;
border-radius: 10%;
Modified: trunk/Source/WebCore/ChangeLog (172007 => 172008)
--- trunk/Source/WebCore/ChangeLog 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/Source/WebCore/ChangeLog 2014-08-04 22:01:04 UTC (rev 172008)
@@ -1,3 +1,28 @@
+2014-08-04 Zalan Bujtas <[email protected]>
+
+ Subpixel rendering: InlineTextBox mistakenly rounds offset value before painting.
+ https://bugs.webkit.org/show_bug.cgi?id=135470
+
+ Reviewed by Simon Fraser.
+
+ This patch removes the premature paint offset adjustment for inlines. Premature snapping
+ could alter the final painting coordinates and push content to wrong positions.
+
+ This patch also enforces WebCore's pixel snapping strategy (round) on text painting.
+ It ensures that text positioning is in sync with other painting related operations including
+ clipping, box decorations etc. Underlying graphics libraries can take different directions on
+ text snapping, for example CG ceils text coordinates vertically (in horizontal context,
+ with the current settings). It can lead to undesired side effects.
+
+ Test: fast/inline/hidpi-inline-selection-leaves-gap.html
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::paint):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::calculateClipRects): wrong direction used at r171896.
+ * rendering/SimpleLineLayoutFunctions.cpp: we don't paint vertical content here.
+ (WebCore::SimpleLineLayout::paintFlow):
+
2014-08-04 Jer Noble <[email protected]>
Unreviewed, rolling out r171992, r171995, & r172000.
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (172007 => 172008)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2014-08-04 22:01:04 UTC (rev 172008)
@@ -506,7 +506,7 @@
LayoutUnit paintEnd = isHorizontal() ? paintInfo.rect.maxX() : paintInfo.rect.maxY();
LayoutUnit paintStart = isHorizontal() ? paintInfo.rect.x() : paintInfo.rect.y();
- FloatPoint adjustedPaintOffset = roundedForPainting(paintOffset, renderer().document().deviceScaleFactor());
+ FloatPoint localPaintOffset(paintOffset);
if (logicalStart >= paintEnd || logicalStart + logicalExtent <= paintStart)
return;
@@ -532,7 +532,7 @@
LayoutUnit widthOfVisibleText = renderer().width(m_start, m_truncation, textPos(), isFirstLine());
LayoutUnit widthOfHiddenText = m_logicalWidth - widthOfVisibleText;
LayoutSize truncationOffset(isLeftToRightDirection() ? widthOfHiddenText : -widthOfHiddenText, 0);
- adjustedPaintOffset.move(isHorizontal() ? truncationOffset : truncationOffset.transposedSize());
+ localPaintOffset.move(isHorizontal() ? truncationOffset : truncationOffset.transposedSize());
}
}
@@ -540,10 +540,10 @@
const RenderStyle& lineStyle = this->lineStyle();
- adjustedPaintOffset.move(0, lineStyle.isHorizontalWritingMode() ? 0 : -logicalHeight());
+ localPaintOffset.move(0, lineStyle.isHorizontalWritingMode() ? 0 : -logicalHeight());
FloatPoint boxOrigin = locationIncludingFlipping();
- boxOrigin.move(adjustedPaintOffset.x(), adjustedPaintOffset.y());
+ boxOrigin.moveBy(localPaintOffset);
FloatRect boxRect(boxOrigin, FloatSize(logicalWidth(), logicalHeight()));
RenderCombineText* combinedText = lineStyle.hasTextCombine() && renderer().isCombineText() && toRenderCombineText(renderer()).isCombined() ? &toRenderCombineText(renderer()) : 0;
@@ -566,12 +566,6 @@
// Set our font.
const Font& font = fontToUse(lineStyle, renderer());
-
- FloatPoint textOrigin = FloatPoint(boxOrigin.x(), boxOrigin.y() + font.fontMetrics().ascent());
-
- if (combinedText)
- combinedText->adjustTextOrigin(textOrigin, boxRect);
-
// 1. Paint backgrounds behind text if needed. Examples of such backgrounds include selection
// and composition underlines.
if (paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseTextClip && !isPrinting) {
@@ -636,6 +630,15 @@
const ShadowData* textShadow = paintInfo.forceBlackText() ? 0 : lineStyle.textShadow();
+ FloatPoint textOrigin(boxOrigin.x(), boxOrigin.y() + font.fontMetrics().ascent());
+ if (combinedText)
+ combinedText->adjustTextOrigin(textOrigin, boxRect);
+
+ if (isHorizontal())
+ textOrigin.setY(roundToDevicePixel(LayoutUnit(textOrigin.y()), renderer().document().deviceScaleFactor()));
+ else
+ textOrigin.setX(roundToDevicePixel(LayoutUnit(textOrigin.x()), renderer().document().deviceScaleFactor()));
+
TextPainter textPainter(*context, paintSelectedTextOnly, paintSelectedTextSeparately, font, sPos, ePos, length, emphasisMark, combinedText, textRun, boxRect, textOrigin, emphasisMarkOffset, textShadow, selectionShadow, isHorizontal(), textPaintStyle, selectionPaintStyle);
textPainter.paintText();
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (172007 => 172008)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2014-08-04 22:01:04 UTC (rev 172008)
@@ -6691,7 +6691,7 @@
LayoutSize subpixelAccumulation = moveOffset - toLayoutSize(LayoutPoint(adjustedPaintOffset));
paintDirtyRect.move(moveOffset);
- paint(context, paintDirtyRect, subpixelAccumulation, paintBehavior, nullptr, paintFlags | PaintLayerTemporaryClipRects);
+ paint(context, paintDirtyRect, LayoutSize(-subpixelAccumulation.width(), -subpixelAccumulation.height()), paintBehavior, nullptr, paintFlags | PaintLayerTemporaryClipRects);
region->restoreRegionObjectsOriginalStyle();
context->restore();
}
Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (172007 => 172008)
--- trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp 2014-08-04 21:58:13 UTC (rev 172007)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp 2014-08-04 22:01:04 UTC (rev 172008)
@@ -79,10 +79,8 @@
GraphicsContextStateSaver stateSaver(context, textPaintStyle.strokeWidth > 0);
updateGraphicsContext(context, textPaintStyle);
- LayoutPoint adjustedPaintOffset = LayoutPoint(roundedForPainting(paintOffset, flow.document().deviceScaleFactor()));
-
LayoutRect paintRect = paintInfo.rect;
- paintRect.moveBy(-adjustedPaintOffset);
+ paintRect.moveBy(-paintOffset);
auto resolver = runResolver(flow, layout);
auto range = resolver.rangeForRect(paintRect);
@@ -92,9 +90,11 @@
continue;
TextRun textRun(run.text());
textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
- context.drawText(font, textRun, run.baseline() + adjustedPaintOffset);
+ FloatPoint textOrigin = run.baseline() + paintOffset;
+ textOrigin.setY(roundToDevicePixel(LayoutUnit(textOrigin.y()), flow.document().deviceScaleFactor()));
+ context.drawText(font, textRun, textOrigin);
if (debugBordersEnabled)
- paintDebugBorders(context, run.rect(), adjustedPaintOffset);
+ paintDebugBorders(context, run.rect(), paintOffset);
}
}
