Add text animation sample; tweak DrawShip sample

Add text animation sample; tweak DrawShip sample

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

Committed: https://skia.googlesource.com/skia/+/629162dd8ef127f793c9cedf0f47a1d4de4d3a3d

[jvanverth1, 2015-11-06 20:04:15]

jvanverth@google.com changed reviewers:
+ bsalomon@google.com, reed@google.com, robertphillips@google.com

[reed1, 2015-11-06 21:11:22]

lgtm, but perhaps Brian should review the GrContext changes

[bsalomon, 2015-11-06 21:23:39]

https://codereview.chromium.org/1410663005/diff/20001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/1410663005/diff/20001/include/gpu/GrContext.h...
include/gpu/GrContext.h:353: void drawFontCache(const SkRect& rect, GrMaskFormat
format, const SkPaint& paint,
Rob, should this be a GrDrawContext function? Either way, let's add some comment
that this is for internal testing only.

[robertphillips, 2015-11-06 21:51:15]

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
File samplecode/SampleAnimatedText.cpp (right):

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:1: /*
2015 ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:30: p.setSubpixelText(true);
Why are we adding 90 here ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:34: 
// This sample demonstrates the cache behavior of bitmap vs. distance field text
// It renders variously sized text with an animated scale and rotation.
// Specifically one should:
//   use 'D' to toggle between bitmap and distance field fonts
//   use '2' to toggle between scaling the image by 2x
//            -- this feature boosts the rendering out of the small point-size
DF-text special case (which falls back to bitmap fonts for small points)

?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:53: switch (uni) {
put in our standard formatting ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:69: 
Isn't this the third instance of this code ? Time to add a class to
sk_tools_utils ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:93: if (grContext) {
Is passing in 'paint' useful ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:108:
paint.setTextSize(SkIntToScalar(i*fSizeScale));
one line ?
remove this loop ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:109: for (SkScalar dx = 0; dx <=
SkIntToScalar(3) / 4;
SK_Scalar1 ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:111: y += paint.getFontSpacing();
Just draw at 110 ?

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:122: bool onAnimate(const SkAnimTimer& timer)
override {
// We add noise to the scale and rotation animations to ...

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:124: fScale += (fScaleInc +
gRand.nextRangeF(-0.025f, 0.025f));
if (fScale >= 2.0f || fScale <= 1.0f) {
   fScaleInc = -fScaleInc;
}

?

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp
File src/gpu/GrTest.cpp (right):

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:111: SkMatrix mat;
reset mat ?

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:114: }
setScaleTranslate ?

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:124: GrClip clip;
don't need reset. Should default to wide open.

[robertphillips, 2015-11-06 21:53:08]

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp
File src/gpu/GrTest.cpp (right):

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:108: SkAutoTUnref<GrDrawContext>
drawContext(this->drawContext(target));
// TODO: add drawContext method to encapsulate this.

[jvanverth1, 2015-11-06 22:06:53]

https://codereview.chromium.org/1410663005/diff/20001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/1410663005/diff/20001/include/gpu/GrContext.h...
include/gpu/GrContext.h:353: void drawFontCache(const SkRect& rect, GrMaskFormat
format, const SkPaint& paint,
On 2015/11/06 21:23:39, bsalomon wrote:
> Rob, should this be a GrDrawContext function? Either way, let's add some
comment
> that this is for internal testing only.

It's in the internal testing functions block (noted above, but not showing
here).

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
File samplecode/SampleAnimatedText.cpp (right):

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:1: /*
On 2015/11/06 21:51:14, robertphillips wrote:
> 2015 ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:30: p.setSubpixelText(true);
On 2015/11/06 21:51:14, robertphillips wrote:
> Why are we adding 90 here ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:34: 
On 2015/11/06 21:51:14, robertphillips wrote:
> // This sample demonstrates the cache behavior of bitmap vs. distance field
text
> // It renders variously sized text with an animated scale and rotation.
> // Specifically one should:
> //   use 'D' to toggle between bitmap and distance field fonts
> //   use '2' to toggle between scaling the image by 2x
> //            -- this feature boosts the rendering out of the small point-size
> DF-text special case (which falls back to bitmap fonts for small points)
> 
> ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:53: switch (uni) {
On 2015/11/06 21:51:14, robertphillips wrote:
> put in our standard formatting ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:69: 
On 2015/11/06 21:51:14, robertphillips wrote:
> Isn't this the third instance of this code ? Time to add a class to
> sk_tools_utils ?

Acknowledged.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:93: if (grContext) {
On 2015/11/06 21:51:14, robertphillips wrote:
> Is passing in 'paint' useful ?

It allows you to change the texture filtering, for one thing.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:108:
paint.setTextSize(SkIntToScalar(i*fSizeScale));
On 2015/11/06 21:51:14, robertphillips wrote:
> one line ?
> remove this loop ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:109: for (SkScalar dx = 0; dx <=
SkIntToScalar(3) / 4;
On 2015/11/06 21:51:14, robertphillips wrote:
> SK_Scalar1 ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:111: y += paint.getFontSpacing();
On 2015/11/06 21:51:14, robertphillips wrote:
> Just draw at 110 ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:122: bool onAnimate(const SkAnimTimer& timer)
override {
On 2015/11/06 21:51:14, robertphillips wrote:
> // We add noise to the scale and rotation animations to ...

Done.

https://codereview.chromium.org/1410663005/diff/20001/samplecode/SampleAnimat...
samplecode/SampleAnimatedText.cpp:124: fScale += (fScaleInc +
gRand.nextRangeF(-0.025f, 0.025f));
On 2015/11/06 21:51:14, robertphillips wrote:
> if (fScale >= 2.0f || fScale <= 1.0f) {
>    fScaleInc = -fScaleInc;
> }
> 
> ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp
File src/gpu/GrTest.cpp (right):

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:108: SkAutoTUnref<GrDrawContext>
drawContext(this->drawContext(target));
On 2015/11/06 21:53:08, robertphillips wrote:
> // TODO: add drawContext method to encapsulate this.

Done.

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:111: SkMatrix mat;
On 2015/11/06 21:51:14, robertphillips wrote:
> reset mat ?

Done.

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:114: }
On 2015/11/06 21:51:14, robertphillips wrote:
> setScaleTranslate ?

Acknowledged.

https://codereview.chromium.org/1410663005/diff/20001/src/gpu/GrTest.cpp#newc...
src/gpu/GrTest.cpp:124: GrClip clip;
On 2015/11/06 21:51:14, robertphillips wrote:
> don't need reset. Should default to wide open.

Done.

[jvanverth1, 2015-11-06 22:07:56]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-11-06 22:07:56]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1410663005/#ps40001
(title: "Add comments")

[commit-bot: I haz the power, 2015-11-06 22:08:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410663005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410663005/40001

[commit-bot: I haz the power, 2015-11-06 22:20:06]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/3b484a40b3be7f0262afadeaf6b741ba5cedcfe1

[jvanverth1, 2015-11-07 00:10:19]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1408063015/ by jvanverth@google.com.

The reason for reverting is: CrOS bots failing..

[jvanverth1, 2015-11-08 15:39:47]

Description was changed from

==========
Add text animation sample; tweak DrawShip sample

Committed:
https://skia.googlesource.com/skia/+/3b484a40b3be7f0262afadeaf6b741ba5cedcfe1
==========

to

==========
Add text animation sample; tweak DrawShip sample

Committed:
https://skia.googlesource.com/skia/+/3b484a40b3be7f0262afadeaf6b741ba5cedcfe1
==========

[jvanverth1, 2015-11-08 15:55:04]

The CQ bit was checked by jvanverth@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-08 15:55:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410663005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410663005/80001

[commit-bot: I haz the power, 2015-11-08 16:05:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-08 16:05:51]

Dry run: This issue passed the CQ dry run.

[jvanverth1, 2015-11-08 16:06:32]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-11-08 16:06:32]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1410663005/#ps80001
(title: "Correct fix for CrOS")

[commit-bot: I haz the power, 2015-11-08 16:06:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410663005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410663005/80001

[commit-bot: I haz the power, 2015-11-08 16:07:30]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/629162dd8ef127f793c9cedf0f47a1d4de4d3a3d