Remove SkPaintOptionsAndroid

Remove SkPaintOptionsAndroid

Committed: https://skia.googlesource.com/skia/+/f32331ffdb5de0440bb337aa7cbdd6f33e9ff23b

Committed: https://skia.googlesource.com/skia/+/3b6255493e458c6b2c1412af908581f0bf3f6b70

[djsollen, 2014-08-06 19:40:29]

This CL can't be submitted until Android is updated to no longer depend on this
class.

https://codereview.chromium.org/447873003/diff/1/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/447873003/diff/1/src/core/SkPaint.cpp#newcode...
src/core/SkPaint.cpp:2187: if (flatFlags &
kHasNonDefaultPaintOptionsAndroid_FlatFlag) {
what is the proper way to document that we can remove this when we get to the
point where we can rev the picture format?

[mtklein, 2014-08-06 19:44:55]

https://codereview.chromium.org/447873003/diff/1/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/447873003/diff/1/src/core/SkPaint.cpp#newcode...
src/core/SkPaint.cpp:2187: if (flatFlags &
kHasNonDefaultPaintOptionsAndroid_FlatFlag) {
I think it's best to bump the picture version.  This should be something like

if (buffer.isVersionLT(kRemovedAndroidPaintOptions_Version &&
    (flatFlags & kHasNonDefaultPaintOptionsAndroid_FlatFlag)) {
   // Skip over old SkPaintOptionsAndroid.
   ...
}

[djsollen, 2014-08-13 17:55:44]

this CL is now ready for review again!

[tfarina, 2014-08-13 18:03:12]

Could you link the android CL here?

[reed1, 2014-08-13 19:30:36]

lgtm

[mtklein, 2014-08-13 19:46:41]

LGTM!

[tomhudson, 2014-08-13 19:52:28]

Code LGTM modulo nits.

https://codereview.chromium.org/447873003/diff/80001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/447873003/diff/80001/src/core/SkPaint.cpp#new...
src/core/SkPaint.cpp:2188: flatFlags &
kHasNonDefaultPaintOptionsAndroid_FlatFlag) {
I suppose hardcoding these is the expedient thing to do?

https://codereview.chromium.org/447873003/diff/80001/src/ports/SkFontConfigPa...
File src/ports/SkFontConfigParser_android.h (right):

https://codereview.chromium.org/447873003/diff/80001/src/ports/SkFontConfigPa...
src/ports/SkFontConfigParser_android.h:60: typedef uint32_t FontVariant;
Why are these a bitfield masquerading as an enum?
If the semantics of default is compact|elegant, shouldn't we be numbering more
cleverly? (It's probably too late for that?)

[djsollen, 2014-08-13 20:00:23]

https://codereview.chromium.org/447873003/diff/80001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/447873003/diff/80001/src/core/SkPaint.cpp#new...
src/core/SkPaint.cpp:2188: flatFlags &
kHasNonDefaultPaintOptionsAndroid_FlatFlag) {
Yep, otherwise I have to keep around a class/file that I just deleted.

https://codereview.chromium.org/447873003/diff/80001/src/ports/SkFontConfigPa...
File src/ports/SkFontConfigParser_android.h (right):

https://codereview.chromium.org/447873003/diff/80001/src/ports/SkFontConfigPa...
src/ports/SkFontConfigParser_android.h:60: typedef uint32_t FontVariant;
On 2014/08/13 19:52:28, tomhudson wrote:
> Why are these a bitfield masquerading as an enum?
> If the semantics of default is compact|elegant, shouldn't we be numbering more
> cleverly? (It's probably too late for that?)

The initial intent was to match the values used by the framework, but it looks
like they are now using 0,1,2 as well.  I'll defer this type of cleanup work to
another CL though.

[djsollen, 2014-08-13 20:00:26]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2014-08-13 20:00:54]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/djsollen@google.com/447873003/80001

[commit-bot: I haz the power, 2014-08-13 20:09:55]

Committed patchset #5 (80001) as f32331ffdb5de0440bb337aa7cbdd6f33e9ff23b

[bsalomon, 2014-08-13 22:17:50]

A revert of this CL (patchset #5) has been created in
https://codereview.chromium.org/473543004/ by bsalomon@google.com.

The reason for reverting is: Breaks the Chromium build:
http://108.170.220.120:10117/builders/Canary-Chrome-Ubuntu13.10-Ninja-x86_64-....

[djsollen, 2014-08-14 13:28:15]

Chrome had the source file defined in 2 places (both in there gypi and ours). 
I've removed the extra one in the CL below so we can now safely land this.

https://codereview.chromium.org/468393002/

[djsollen, 2014-08-14 13:28:22]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2014-08-14 13:28:52]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/djsollen@google.com/447873003/80001

[commit-bot: I haz the power, 2014-08-14 13:29:14]

Committed patchset #5 (80001) as 3b6255493e458c6b2c1412af908581f0bf3f6b70