Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(425)

Issue 881133003: Revert of text-combine should scale rather than fall back to none when wide (Closed)

Created:
5 years, 10 months ago by vsevik
Modified:
5 years, 10 months ago
CC:
blink-reviews, blink-reviews-rendering, zoltan1, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, slimming-paint-reviews_chromium.org, jchaffraix+rendering, blink-reviews-paint_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Revert of text-combine should scale rather than fall back to none when wide (patchset #5 id:180001 of https://codereview.chromium.org/799123003/) Reason for revert: Makes fast/text/text-combine-selection-crash.html crash on mac: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Ftext%2Ftext-combine-selection-crash.html&testType=layout-tests Original issue's description: > text-combine should scale rather than fall back to none when wide > > This patch follows the CSS spec change in 2014 CR: > Old: Combine if vertical flow and its width <= ~1em > Current: Combine if vertical flow. Scale if width > ~1em > > CR: http://www.w3.org/TR/css-writing-modes-3/#text-combine > ED: http://dev.w3.org/csswg/css-writing-modes/#text-combine > > In addition to the changes to RenderCombineText, this spec change has a > small but pleasant side effect to our implementation. The > RenderCombinedText delays the width calculation until it's needed. But the > spec also says that it has to be treated as an U+FFFC OBJECT REPLACEMENT > CHARACTER to line breakers when combined. > > From these two facts, line breakers must be aware of RenderCombinedText and > update it (combineText()) when needed. Also, the fact that combineText() > could change the length of text brought additional complexity. We have a > few calls to combineText() in line breakers, and still has some flakiness > such as bug 312606. The new spec eliminates this complexity, so our line > breakers are a bit cleaner, faster, and less flaky. > > fast/text/font-variant-width.html had to be rewritten because it relied on > the behavior of text-combine to fallback when wide. > > BUG=433176, 312606 > TEST=fast/writing-mode/text-combine-compress.html > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189235 TBR=kouhei@chromium.org,leviw@chromium.org,kojii@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=433176, 312606

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -311 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +1 line, -7 lines 0 comments Download
M LayoutTests/fast/dynamic/text-combine.html View 1 chunk +3 lines, -3 lines 0 comments Download
M LayoutTests/fast/text/font-variant-width.html View 2 chunks +37 lines, -43 lines 0 comments Download
M LayoutTests/fast/text/font-variant-width-expected.txt View 1 chunk +22 lines, -14 lines 0 comments Download
M LayoutTests/fast/text/international/combine-at-line-break-crash.html View 1 chunk +2 lines, -2 lines 0 comments Download
D LayoutTests/fast/writing-mode/text-combine-compress.html View 1 chunk +0 lines, -105 lines 0 comments Download
D LayoutTests/fast/writing-mode/text-combine-compress-expected.txt View 1 chunk +0 lines, -43 lines 0 comments Download
A LayoutTests/virtual/antialiasedtext/fast/text/font-variant-width-expected.txt View 1 chunk +25 lines, -0 lines 0 comments Download
M Source/core/paint/InlineTextBoxPainter.cpp View 1 chunk +5 lines, -15 lines 0 comments Download
M Source/core/paint/TextPainter.cpp View 1 chunk +1 line, -8 lines 0 comments Download
M Source/core/rendering/RenderBlockLineLayout.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderCombineText.h View 2 chunks +1 line, -5 lines 0 comments Download
M Source/core/rendering/RenderCombineText.cpp View 5 chunks +38 lines, -66 lines 0 comments Download
M Source/core/rendering/line/BreakingContextInlineHeaders.h View 2 chunks +16 lines, -0 lines 0 comments Download
M Source/core/rendering/line/LineBreaker.cpp View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
vsevik
Created Revert of text-combine should scale rather than fall back to none when wide
5 years, 10 months ago (2015-01-30 09:47:38 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/881133003/1
5 years, 10 months ago (2015-01-30 09:47:57 UTC) #2
commit-bot: I haz the power
5 years, 10 months ago (2015-01-30 09:48:09 UTC) #4
Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 728.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
01f7aea182f67a431d27a4f6781ec2bc01e92ce1..22ff77ca42f0d64c62b959edb9f2360103a219c2
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -728,13 +728,7 @@
 
 crbug.com/311469 [ Mac ] svg/zoom/page/zoom-zoom-coords.xhtml [ Failure Pass ]
 
-crbug.com/433176 fast/dynamic/text-combine.html [ NeedsRebaseline ]
-crbug.com/433176 fast/text/decorations-with-text-combine.html [ NeedsRebaseline
]
-crbug.com/433176 fast/text/international/text-combine-image-test.html [
NeedsRebaseline ]
-crbug.com/433176
virtual/antialiasedtext/fast/text/decorations-with-text-combine.html [
NeedsRebaseline ]
-crbug.com/433176
virtual/antialiasedtext/fast/text/international/text-combine-image-test.html [
NeedsRebaseline ]
-crbug.com/433176
virtual/slimmingpaint/fast/text/decorations-with-text-combine.html [
NeedsRebaseline ]
-crbug.com/433176
virtual/slimmingpaint/fast/text/international/text-combine-image-test.html [
NeedsRebaseline ]
+crbug.com/312606 [ Mac ] fast/text/international/text-combine-image-test.html [
Failure Pass ]
 
 crbug.com/312613 fast/canvas/canvas-blending-pattern-over-gradient.html [ Pass
Timeout ]

Powered by Google App Engine
This is Rietveld 408576698