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

Issue 1933393002: Move SkTypeface to sk_sp. (Closed)

Created:
4 years, 7 months ago by bungeman-skia
Modified:
4 years, 7 months ago
Reviewers:
reed1, f(malita), tomhudson
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Make vc++ happy. #

Total comments: 18

Patch Set 3 : Address comments. #

Total comments: 36

Patch Set 4 : More comments and rebase. #

Patch Set 5 : Rebase. #

Total comments: 1

Patch Set 6 : Restore deleted Android code. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+331 lines, -420 lines) Patch
M bench/CmapBench.cpp View 1 chunk +1 line, -1 line 0 comments Download
M bench/SkGlyphCacheBench.cpp View 3 chunks +2 lines, -7 lines 0 comments Download
M bench/TextBench.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M bench/TextBlobBench.cpp View 2 chunks +5 lines, -7 lines 0 comments Download
M dm/DM.cpp View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M fuzz/FilterFuzz.cpp View 1 chunk +1 line, -3 lines 0 comments Download
M gm/coloremoji.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M gm/colortype.cpp View 1 2 3 3 chunks +5 lines, -12 lines 0 comments Download
M gm/colortypexfermode.cpp View 1 2 3 3 chunks +6 lines, -13 lines 0 comments Download
M gm/dftext.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M gm/fontcache.cpp View 2 chunks +2 lines, -10 lines 0 comments Download
M gm/fontmgr.cpp View 6 chunks +10 lines, -10 lines 0 comments Download
M gm/fontscalerdistortable.cpp View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M gm/gammatext.cpp View 1 2 chunks +3 lines, -8 lines 0 comments Download
M gm/mixedtextblobs.cpp View 2 chunks +4 lines, -4 lines 0 comments Download
M gm/pdf_never_embed.cpp View 1 2 3 1 chunk +5 lines, -6 lines 0 comments Download
M gm/poly2poly.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M gm/textblob.cpp View 2 chunks +4 lines, -4 lines 0 comments Download
M gm/textblobmixedsizes.cpp View 1 2 3 2 chunks +1 line, -2 lines 0 comments Download
M gm/textblobrandomfont.cpp View 1 2 2 chunks +6 lines, -9 lines 0 comments Download
M gm/typeface.cpp View 2 chunks +2 lines, -8 lines 0 comments Download
M gm/variedtext.cpp View 5 chunks +10 lines, -18 lines 0 comments Download
M gm/verttext2.cpp View 3 chunks +7 lines, -15 lines 0 comments Download
M gyp/skia_for_android_framework_defines.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M include/core/SkFont.h View 2 chunks +9 lines, -9 lines 0 comments Download
M include/core/SkPaint.h View 1 chunk +3 lines, -1 line 0 comments Download
M include/core/SkTypeface.h View 1 2 3 3 chunks +56 lines, -31 lines 0 comments Download
M include/ports/SkFontConfigInterface.h View 1 2 3 4 1 chunk +6 lines, -3 lines 0 comments Download
M public.bzl View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M samplecode/ClockFaceView.cpp View 2 chunks +3 lines, -8 lines 0 comments Download
M samplecode/SampleAll.cpp View 1 chunk +1 line, -2 lines 0 comments Download
M samplecode/SampleAnimatedText.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleApp.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleApp.cpp View 1 2 3 4 2 chunks +1 line, -2 lines 0 comments Download
M samplecode/SampleFilterFuzz.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
M samplecode/SampleFontScalerTest.cpp View 2 chunks +3 lines, -10 lines 0 comments Download
M samplecode/SampleSlides.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleText.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleXfermodesBlur.cpp View 2 chunks +2 lines, -5 lines 0 comments Download
M src/animator/SkDrawPaint.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/animator/SkPaintPart.h View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M src/core/SkFont.cpp View 5 chunks +13 lines, -22 lines 0 comments Download
M src/core/SkPaint.cpp View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
M src/core/SkPictureData.cpp View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M src/core/SkTextBlob.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/SkTypeface.cpp View 1 2 3 4 6 chunks +22 lines, -23 lines 0 comments Download
M src/core/SkTypefacePriv.h View 1 chunk +5 lines, -5 lines 0 comments Download
M src/fonts/SkGScalerContext.h View 2 chunks +4 lines, -5 lines 0 comments Download
M src/fonts/SkGScalerContext.cpp View 1 chunk +4 lines, -7 lines 0 comments Download
M src/fonts/SkRandomScalerContext.h View 2 chunks +5 lines, -6 lines 0 comments Download
M src/fonts/SkRandomScalerContext.cpp View 1 chunk +2 lines, -6 lines 0 comments Download
M src/pdf/SkPDFFont.cpp View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/ports/SkFontMgr_android.cpp View 1 2 3 4 5 3 chunks +7 lines, -7 lines 0 comments Download
M src/svg/SkSVGDevice.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M src/utils/SkLua.cpp View 1 2 3 4 2 chunks +4 lines, -5 lines 0 comments Download
M src/utils/SkWhitelistTypefaces.cpp View 1 2 3 6 chunks +10 lines, -10 lines 0 comments Download
M tests/FontHostStreamTest.cpp View 1 2 3 2 chunks +6 lines, -13 lines 0 comments Download
M tests/FontHostTest.cpp View 1 2 3 6 chunks +13 lines, -14 lines 0 comments Download
M tests/FontMgrTest.cpp View 4 chunks +5 lines, -7 lines 0 comments Download
M tests/FontObjTest.cpp View 3 chunks +3 lines, -4 lines 0 comments Download
M tests/PDFPrimitivesTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M tests/PaintTest.cpp View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M tests/PictureTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M tests/SerializationTest.cpp View 1 2 3 4 3 chunks +6 lines, -6 lines 0 comments Download
M tests/TextBlobCacheTest.cpp View 1 2 3 4 5 1 chunk +2 lines, -3 lines 0 comments Download
M tests/TextBlobTest.cpp View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M tests/TypefaceTest.cpp View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M tools/Resources.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/Resources.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M tools/debugger/SkDrawCommand.cpp View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M tools/sk_tool_utils.h View 3 chunks +3 lines, -3 lines 0 comments Download
M tools/sk_tool_utils.cpp View 3 chunks +7 lines, -11 lines 0 comments Download
M tools/sk_tool_utils_font.cpp View 3 chunks +4 lines, -3 lines 0 comments Download
M tools/using_skia_and_harfbuzz.cpp View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 45 (21 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/1
4 years, 7 months ago (2016-04-29 19:25:36 UTC) #3
commit-bot: I haz the power
Dry run: 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_64-Debug-Trybot/builds/8299)
4 years, 7 months ago (2016-04-29 19:29:08 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/20001
4 years, 7 months ago (2016-04-29 19:41:54 UTC) #7
bungeman-skia
I'm giving up for the moment on writing an efficient smart pointer system for c++, ...
4 years, 7 months ago (2016-04-29 19:53:25 UTC) #9
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-04-29 20:11:53 UTC) #11
tomhudson
Disturbed by how often we're using sk_ref_sp(), since on the Chrome side that usually means ...
4 years, 7 months ago (2016-04-29 21:10:36 UTC) #12
bungeman-skia
With this change there are now 163 instances of sk_ref_sp, this add 12. https://codereview.chromium.org/1933393002/diff/20001/gm/fontmgr.cpp File ...
4 years, 7 months ago (2016-04-29 22:03:24 UTC) #13
reed1
https://codereview.chromium.org/1933393002/diff/40001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): https://codereview.chromium.org/1933393002/diff/40001/include/core/SkRefCnt.h#newcode333 include/core/SkRefCnt.h:333: skstd::add_lvalue_reference_t<T> operator*() const { Is this change a necessary ...
4 years, 7 months ago (2016-05-01 18:53:52 UTC) #14
tomhudson
Might be worth landing as-is, but Mike's right that the job's not done until this ...
4 years, 7 months ago (2016-05-02 12:26:08 UTC) #15
f(malita)
lgtm w/ nits, modulo other reviewers. https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp File gm/colortype.cpp (right): https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp#newcode30 gm/colortype.cpp:30: auto orig = ...
4 years, 7 months ago (2016-05-02 13:46:36 UTC) #16
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/60001
4 years, 7 months ago (2016-05-02 20:06:33 UTC) #18
bungeman-skia
https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp File gm/colortype.cpp (right): https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp#newcode30 gm/colortype.cpp:30: auto orig = sk_tool_utils::create_portable_typeface("serif", SkTypeface::kBold); On 2016/05/02 13:46:35, f(malita) ...
4 years, 7 months ago (2016-05-02 20:24:55 UTC) #21
reed1
https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp File gm/colortype.cpp (right): https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp#newcode30 gm/colortype.cpp:30: auto orig = sk_tool_utils::create_portable_typeface("serif", SkTypeface::kBold); On 2016/05/02 13:46:35, f(malita) ...
4 years, 7 months ago (2016-05-03 13:45:00 UTC) #22
f(malita)
On 2016/05/03 13:45:00, reed1 wrote: > https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp > File gm/colortype.cpp (right): > > https://codereview.chromium.org/1933393002/diff/40001/gm/colortype.cpp#newcode30 > ...
4 years, 7 months ago (2016-05-03 14:02:28 UTC) #23
bungeman-skia
reed, you ok with this?
4 years, 7 months ago (2016-05-10 17:50:48 UTC) #24
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/80001
4 years, 7 months ago (2016-05-10 18:54:02 UTC) #27
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-10 19:16:14 UTC) #29
reed1
lgtm
4 years, 7 months ago (2016-05-11 18:59:14 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/80001
4 years, 7 months ago (2016-05-11 19:03:12 UTC) #33
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/6296da736fbf40aae881650c239420f64e576c3f
4 years, 7 months ago (2016-05-11 19:38:23 UTC) #36
scroggo
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/1974783002/ by scroggo@google.com. ...
4 years, 7 months ago (2016-05-12 13:22:05 UTC) #37
bungeman-skia
https://codereview.chromium.org/1933393002/diff/80001/src/ports/SkFontMgr_android.cpp File src/ports/SkFontMgr_android.cpp (left): https://codereview.chromium.org/1933393002/diff/80001/src/ports/SkFontMgr_android.cpp#oldcode390 src/ports/SkFontMgr_android.cpp:390: if (glyphID != 0) { How did this sneak ...
4 years, 7 months ago (2016-05-12 16:47:41 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1933393002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1933393002/100001
4 years, 7 months ago (2016-05-12 16:48:11 UTC) #43
commit-bot: I haz the power
4 years, 7 months ago (2016-05-12 17:09:36 UTC) #45
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/13b9c95295f4c5732e34574789e721a6bc08f7b4

Powered by Google App Engine
This is Rietveld 408576698