Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2350453002:
GM: add test for type1 font (Closed)
|
Created:
4 years, 3 months ago by hal.canary Modified:
4 years, 3 months ago Reviewers:
bungeman-skia CC:
reviews_skia.org Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionGM: add test for type1 font
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2350453002
Committed: https://skia.googlesource.com/skia/+/4ecf0d484373b806b41f7ff1747ed380ee3be817
Patch Set 1 #Patch Set 2 : move to gm/typeface.cpp #Patch Set 3 : use typefacerendering gm #
Total comments: 6
Patch Set 4 : comments from bungeman@ #
Messages
Total messages: 26 (16 generated)
Description was changed from ========== GM: add test for type1 font BUG=skia: ========== to ========== GM: add test for type1 font BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2350453002 ==========
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
halcanary@google.com changed reviewers: + bungeman@google.com
the coverage tool says this is a completely untested code path.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This looks like something that should be added to TypefaceRenderingGM which is designed to be the GM for covering as many code paths for rasterizing as possible. The idea is to have one GM to look at for all the various settings.
On 2016/09/16 20:02:59, bungeman-skia wrote: > This looks like something that should be added to TypefaceRenderingGM which is > designed to be the GM for covering as many code paths for rasterizing as > possible. The idea is to have one GM to look at for all the various settings. done
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp File gm/typeface.cpp (right): https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode157 gm/typeface.cpp:157: /////////////////////////////////////////////////////////////////////////////// Probably should leave this line out; these DEF_GM do go with the above code. https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode281 gm/typeface.cpp:281: DEF_SIMPLE_GM_BG_NAME(type1pfa_typefacerendering, canvas, 640, 680, SK_ColorWHITE, Just to make things line up in tools and such, this name should be typefacerendering_pfa (I don't think we need to state type1 here, the font format pfa only does type1). (Same below with pfb.) https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode283 gm/typeface.cpp:283: sk_tool_utils::major_platform_os_name().c_str())) { Here also, to make the names line up "typefacerendering_pfa%"
https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp File gm/typeface.cpp (right): https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode157 gm/typeface.cpp:157: /////////////////////////////////////////////////////////////////////////////// On 2016/09/19 17:24:30, bungeman-skia wrote: > Probably should leave this line out; these DEF_GM do go with the above code. Done. https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode281 gm/typeface.cpp:281: DEF_SIMPLE_GM_BG_NAME(type1pfa_typefacerendering, canvas, 640, 680, SK_ColorWHITE, On 2016/09/19 17:24:30, bungeman-skia wrote: > Just to make things line up in tools and such, this name should be > typefacerendering_pfa (I don't think we need to state type1 here, the font > format pfa only does type1). (Same below with pfb.) Done. https://codereview.chromium.org/2350453002/diff/40001/gm/typeface.cpp#newcode283 gm/typeface.cpp:283: sk_tool_utils::major_platform_os_name().c_str())) { On 2016/09/19 17:24:30, bungeman-skia wrote: > Here also, to make the names line up "typefacerendering_pfa%" Done.
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== GM: add test for type1 font BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2350453002 ========== to ========== GM: add test for type1 font BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2350453002 Committed: https://skia.googlesource.com/skia/+/4ecf0d484373b806b41f7ff1747ed380ee3be817 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/4ecf0d484373b806b41f7ff1747ed380ee3be817
Message was sent while issue was closed.
On 2016/09/20 20:11:04, commit-bot: I haz the power wrote: > Committed patchset #4 (id:60001) as > https://skia.googlesource.com/skia/+/4ecf0d484373b806b41f7ff1747ed380ee3be817 https://luci-milo.appspot.com/swarming/task/31624773e24add10/steps/dm/0/stdout Failures: serialize-8888 gm typefacerendering_pfaMac: Pixels don't match reference 1 failures
Message was sent while issue was closed.
On 2016/09/20 21:33:37, fmalita_google_do_not_use wrote: > On 2016/09/20 20:11:04, commit-bot: I haz the power wrote: > > Committed patchset #4 (id:60001) as > > https://skia.googlesource.com/skia/+/4ecf0d484373b806b41f7ff1747ed380ee3be817 > > https://luci-milo.appspot.com/swarming/task/31624773e24add10/steps/dm/0/stdout > > Failures: > serialize-8888 gm typefacerendering_pfaMac: Pixels don't match reference > 1 failures Heh, interesting. I don't think we can serialize these fonts on Mac the way things are currently set up. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
