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

Issue 763973003: Correct font not selected to calculate text run width (Closed)

Created:
6 years ago by h.joshi
Modified:
6 years ago
CC:
blink-reviews, blink-reviews-rendering, zoltan1, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering
Base URL:
https://chromium.googlesource.com/chromium/blink.git@diff_font_render
Project:
blink
Visibility:
Public.

Description

Correct font not selected to calculate text run width Original patch: https://codereview.chromium.org/680213004 Breaking original patch into two parts for better tracking. In RenderText.cpp, when width of text run is calculated sometimes different fonts are used to get width of string which is different than the font used to draw text. This happens due to fallback font architecture which results in font change. We need to make call to calculate width only when primary font has a glyph for the requested codepoint. BUG=416425 TEST=fast/css/font-face-opentype.html, fast/text/line-break-after-question-mark.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187387

Patch Set 1 #

Total comments: 4

Patch Set 2 : Comment fixes #

Patch Set 3 : Updating Test Expectation file #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -4 lines) Patch
M LayoutTests/TestExpectations View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/css/font-face-opentype-expected.png View Binary file 0 comments Download
M Source/core/rendering/RenderText.cpp View 1 2 2 chunks +12 lines, -3 lines 0 comments Download

Messages

Total messages: 23 (9 generated)
Dominik Röttsches
Please update your change description to contain a one-line descriptive headline with what functional change ...
6 years ago (2014-12-15 09:49:00 UTC) #3
h.joshi
@Dominik: Thank you for the review. Suggested changes are made. https://codereview.chromium.org/763973003/diff/1/LayoutTests/TestExpectations File LayoutTests/TestExpectations (right): https://codereview.chromium.org/763973003/diff/1/LayoutTests/TestExpectations#newcode1452 ...
6 years ago (2014-12-16 07:02:20 UTC) #6
Dominik Röttsches
"Correct font not selected to calculate text run width" works as a headline. Please put ...
6 years ago (2014-12-16 11:53:36 UTC) #7
h.joshi
> "Correct font not selected to calculate text run width" works as a headline. > ...
6 years ago (2014-12-16 13:15:11 UTC) #8
Dominik Röttsches
It's usually a good idea to trigger trybots during development of your patch. It helps ...
6 years ago (2014-12-16 14:21:24 UTC) #9
h.joshi
@Dominik: Thank you, I have trybots access, Will make sure to run trybots on all ...
6 years ago (2014-12-16 16:59:01 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/763973003/60001
6 years ago (2014-12-17 10:11:22 UTC) #12
commit-bot: I haz the power
Failed to apply patch for LayoutTests/TestExpectations: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years ago (2014-12-17 10:12:18 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/763973003/80001
6 years ago (2014-12-17 10:22:09 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/22530)
6 years ago (2014-12-17 10:28:01 UTC) #18
h.joshi
@Emil: Pls review, getting following error ** Presubmit ERRORS ** Missing LGTM from an OWNER ...
6 years ago (2014-12-17 10:34:35 UTC) #19
eae
On 2014/12/17 10:34:35, h.joshi wrote: > @Emil: Pls review, getting following error > ** Presubmit ...
6 years ago (2014-12-17 17:08:04 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/763973003/80001
6 years ago (2014-12-17 17:09:02 UTC) #22
commit-bot: I haz the power
6 years ago (2014-12-17 17:12:44 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187387

Powered by Google App Engine
This is Rietveld 408576698