Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1711053004:
Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. (Closed)
|
Created:
4 years, 10 months ago by dogben Modified:
4 years, 10 months ago Reviewers:
bungeman-skia CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionChange type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1711053004
Committed: https://skia.googlesource.com/skia/+/4534562f7600e036fd7561df1fe04dc5e58a58fc
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix scaleGlyphMetrics; reposition args to SkDEBUGF. #Messages
Total messages: 20 (9 generated)
Description was changed from ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. BUG=skia: ========== to ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by benjaminwagner@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711053004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711053004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
benjaminwagner@google.com changed reviewers: + bungeman@google.com
On 2016/02/19 at 17:49:35, Ben Wagner wrote: > I'm confused by the diffs on gold. I will investigate more.
https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... File src/ports/SkFontHost_FreeType.cpp (right): https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... src/ports/SkFontHost_FreeType.cpp:913: fFace->family_name, SkFDot6ToScalar(fScaleY))); The odd spacing is to line up the variables under the associated substitution. https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... src/ports/SkFontHost_FreeType.cpp:1192: scaleGlyphMetrics(*glyph, SkFDot6ToScalar(fScaleY) / fFace->size->metrics.y_ppem); I'm pretty sure the gm change is due to this now having less precision.
The CQ bit was checked by benjaminwagner@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711053004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711053004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... File src/ports/SkFontHost_FreeType.cpp (right): https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... src/ports/SkFontHost_FreeType.cpp:913: fFace->family_name, SkFDot6ToScalar(fScaleY))); On 2016/02/19 at 20:28:10, bungeman1 wrote: > The odd spacing is to line up the variables under the associated substitution. Fixed. https://codereview.chromium.org/1711053004/diff/1/src/ports/SkFontHost_FreeTy... src/ports/SkFontHost_FreeType.cpp:1192: scaleGlyphMetrics(*glyph, SkFDot6ToScalar(fScaleY) / fFace->size->metrics.y_ppem); On 2016/02/19 at 20:28:09, bungeman1 wrote: > I'm pretty sure the gm change is due to this now having less precision. Fixed.
Description was changed from ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by bungeman@google.com
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711053004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711053004/20001
Message was sent while issue was closed.
Description was changed from ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Change type of SkScalerContext_FreeType::fScaleX/Y. All uses want FT_F26Dot6. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/4534562f7600e036fd7561df1fe04dc5e58a58fc ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/4534562f7600e036fd7561df1fe04dc5e58a58fc |
