Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1982303002:
Make GrFontScaler not be ref-counted (Closed)
DescriptionMake GrFontScaler not be ref-counted
This object is made invalid when the SkGlyphCache that it points to is destroyed.
BUG=chromium:608566
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1982303002
Committed: https://skia.googlesource.com/skia/+/4bb7ce7b3d64841bed5a4e8e71f2a8e79d3db413
Patch Set 1 #Patch Set 2 : update INHERITED #
Total comments: 1
Dependent Patchsets: Messages
Total messages: 21 (9 generated)
Description was changed from ========== Make GrFontScaler not be ref-counted This object is made invalid when the SkGlyphCache that it points to is destroyed. BUG=chromium:608566 ========== to ========== Make GrFontScaler not be ref-counted This object is made invalid when the SkGlyphCache that it points to is destroyed. BUG=chromium:608566 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
bsalomon@google.com changed reviewers: + bungeman@google.com, robertphillips@google.com
The CQ bit was checked by bsalomon@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1982303002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1982303002/1
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2016-05-17 19:11 UTC
lgtm
The CQ bit was checked by bsalomon@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1982303002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1982303002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
This lgtm, but possible additional cleanup for the future... https://codereview.chromium.org/1982303002/diff/20001/src/gpu/text/GrTextUtil... File src/gpu/text/GrTextUtils.cpp (right): https://codereview.chromium.org/1982303002/diff/20001/src/gpu/text/GrTextUtil... src/gpu/text/GrTextUtils.cpp:578: GrFontScaler* GrTextUtils::GetGrFontScaler(SkGlyphCache* cache) { It actually seems strange that this is in GrTextUtils as well, since this is the only valid way to call the constructor for GrFontScaler (it's a factory). It seems like this function should be named GrFontScaler::Get(SkGlyphCache* strike) and the GrFontScaler constructor be made private.
The CQ bit was checked by bungeman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1982303002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1982303002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
The CQ bit was checked by borenet@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1982303002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1982303002/20001
Message was sent while issue was closed.
Description was changed from ========== Make GrFontScaler not be ref-counted This object is made invalid when the SkGlyphCache that it points to is destroyed. BUG=chromium:608566 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Make GrFontScaler not be ref-counted This object is made invalid when the SkGlyphCache that it points to is destroyed. BUG=chromium:608566 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/4bb7ce7b3d64841bed5a4e8e71f2a8e79d3db413 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/4bb7ce7b3d64841bed5a4e8e71f2a8e79d3db413
Message was sent while issue was closed.
On 2016/05/17 14:14:05, bungeman-skia wrote: > This lgtm, but possible additional cleanup for the future... > > https://codereview.chromium.org/1982303002/diff/20001/src/gpu/text/GrTextUtil... > File src/gpu/text/GrTextUtils.cpp (right): > > https://codereview.chromium.org/1982303002/diff/20001/src/gpu/text/GrTextUtil... > src/gpu/text/GrTextUtils.cpp:578: GrFontScaler* > GrTextUtils::GetGrFontScaler(SkGlyphCache* cache) { > It actually seems strange that this is in GrTextUtils as well, since this is the > only valid way to call the constructor for GrFontScaler (it's a factory). It > seems like this function should be named GrFontScaler::Get(SkGlyphCache* strike) > and the GrFontScaler constructor be made private. There will definitely be more cleanup coming... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
