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

Issue 110313003: Cleanup mac specific TextLayout measurement helper (Closed)

Created:
7 years ago by Dominik Röttsches
Modified:
6 years, 2 months ago
CC:
blink-reviews, jamesr, krit, bemjb+rendering_chromium.org, dsinclair, zoltan1, eae+blinkwatch, leviw+renderwatch, jbroman, danakj, Rik, jchaffraix+rendering, Stephen Chennney, pdr., rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Cleanup mac specific TextLayout measurement helper As far as my analysis goes the rebaselines are needed due to the different kerning behaviour. The textLayout based measurement returns kerned values when measuring substrings of a run. When not using textLayout, we create a new run and pass that to Font::width which then returns a value that is not kerned for only the last character in the run. The new behaviour is consistent with the behaviour on non-mac, thus I don't see an issue with changing this. Additionally, RenderText::width and ::widthFromCache behave the same way, creating new runs for measurement. BUG=329543 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164915

Patch Set 1 #

Patch Set 2 : Cleanup mac specific TextLayout measurement helper - Debug code removed #

Patch Set 3 : Cleanup mac specific TextLayout measurement helper - including Rebaselines #

Patch Set 4 : Cleanup mac specific TextLayout measurement helper - Using bot rebaselines #

Patch Set 5 : Cleanup mac specific TextLayout measurement helper - Using bot rebaselines (rebased) #

Patch Set 6 : Cleanup mac specific TextLayout measurement helper - More rebaselining #

Patch Set 7 : Cleanup mac specific TextLayout measurement helper - Baseline search path cleanup #

Patch Set 8 : Cleanup mac specific TextLayout measurement helper - Additional pixel baselines #

Patch Set 9 : Adding rebaselines, minor rounding issues #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -0 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 1 chunk +62 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Dominik Röttsches
6 years, 11 months ago (2014-01-09 15:41:34 UTC) #1
eae
LGTM
6 years, 11 months ago (2014-01-09 16:57:06 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dominik.rottsches@intel.com/110313003/380001
6 years, 11 months ago (2014-01-10 23:36:51 UTC) #3
commit-bot: I haz the power
Change committed as 164915
6 years, 11 months ago (2014-01-11 01:34:46 UTC) #4
Dominik Röttsches
6 years, 11 months ago (2014-01-14 13:29:35 UTC) #5
Message was sent while issue was closed.
Nice, the perf dashboard seems to appreciate the change:

Delta	Revision Range	Bot	Test Suite	Test
129.4%	244366-244392	chromium-rel-mac6	blink_perf	Layout_ArabicLineLayout
100.0%	244426-244427	chromium-rel-mac8	blink_perf	Layout_ArabicLineLayout

https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=chromium-rel-...

Powered by Google App Engine
This is Rietveld 408576698