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

Issue 860163004: Using FloatRect in SimpleShaper and remove GlyphBounds (Closed)

Created:
5 years, 11 months ago by h.joshi
Modified:
5 years, 10 months ago
CC:
blink-reviews, Dominik Röttsches, krit, Rik, jbroman, danakj, pdr+graphicswatchlist_chromium.org, f(malita), Stephen Chennney, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Use FloatRect in SimpleShaper and remove GlyphBounds. GlyphBounds is only used in SimpleShaper. Removing GlyphBounds class and making consistency between SimpleShapper and HarfbuzzShaper code. BUG=444921 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189386

Patch Set 1 #

Patch Set 2 : Updating TestExpectation with virtual test #

Patch Set 3 : Comment fixes #

Messages

Total messages: 20 (5 generated)
h.joshi
Pls review. Windows bots are failing in different test case, tried 3 times and all ...
5 years, 11 months ago (2015-01-22 06:57:44 UTC) #2
eae
I like the change to use a FloatRect instead of the one-off class, always computing ...
5 years, 11 months ago (2015-01-22 14:47:45 UTC) #3
jbroman
I don't believe this is correct for RTL. In that case, we shape as though ...
5 years, 11 months ago (2015-01-22 15:21:22 UTC) #5
h.joshi
@Emil: We can change the implementation and pass pointer of FlaotRect and then use the ...
5 years, 11 months ago (2015-01-22 18:10:58 UTC) #6
h.joshi
On 2015/01/22 18:10:58, h.joshi wrote: > @Emil: We can change the implementation and pass pointer ...
5 years, 11 months ago (2015-01-23 08:33:05 UTC) #7
h.joshi
last comment went without any thing added, sorry for that. @Emil: Made changes to code ...
5 years, 11 months ago (2015-01-23 08:36:12 UTC) #8
h.joshi
@jbroman: We have following code flow for reversing: if (runInfo.run.rtl()) { // Glyphs are shaped ...
5 years, 10 months ago (2015-01-29 12:36:40 UTC) #9
jbroman
Hmm, it's possible it isn't making things any worse for RTL. OK, then, I won't ...
5 years, 10 months ago (2015-02-02 15:47:18 UTC) #10
eae
LGTM Thank you!
5 years, 10 months ago (2015-02-02 21:45:46 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/860163004/40001
5 years, 10 months ago (2015-02-02 21:46:45 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/48883)
5 years, 10 months ago (2015-02-02 23:33:21 UTC) #15
h.joshi
Thank you Email and jbroman.
5 years, 10 months ago (2015-02-03 05:05:41 UTC) #16
h.joshi
Thank you Emil and jbroman.
5 years, 10 months ago (2015-02-03 05:05:59 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/860163004/40001
5 years, 10 months ago (2015-02-03 05:15:21 UTC) #19
commit-bot: I haz the power
5 years, 10 months ago (2015-02-03 06:06:03 UTC) #20
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189386

Powered by Google App Engine
This is Rietveld 408576698