Enable automatic rescaling in bench_pictures

Enable automatic rescaling in bench_pictures

bench_pictures with "--device gpu" is failing because we're trying to allocate
too much GPU memory.  Move the recently-added scaling code into picture_utils
and share it between render_pictures and bench_pictures.
Committed: https://code.google.com/p/skia/source/detail?r=5543

[Leon, 2012-09-13 21:31:40]

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp
File tools/bench_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp#n...
tools/bench_pictures_main.cpp:112: SkPicture* picture = new
SkPicture(&inputStream);
SkNEW_ARGS.

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp#n...
tools/bench_pictures_main.cpp:120: picture->height(), filename.c_str());
Should we print this after doing the resize?

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp
File tools/picture_utils.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode117
tools/picture_utils.cpp:117: SkPicture* resize_if_needed(SkPicture* picture,
SkAutoUnref* aur) {
Would it make more sense to take SkAutoTUnref<SkPicture>?

There may be good reasons to do it as is, but it feels odd to me that you are
passing in an SkAutoUnref and the SkPicture it owns.

At the end of this function, aur points to the new picture, so you could even
get away with returning void and calling aur to get to the picture.

Again, it just feels odd to me that there are multiple ways to get at the same
data.

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode121
tools/picture_utils.cpp:121: pic = new SkPicture();
SkNEW

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode122
tools/picture_utils.cpp:122: picture->ref();
Won't we need to unref this?

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.h
File tools/picture_utils.h (right):

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.h#newcode53
tools/picture_utils.h:53: // and returns a new SkPicture.  Otherwise, the given
SkPicture is returned.
Could you expand on this comment? At the end of this function, aur now points to
the new picture that is returned (if anything has changed).

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.h#newcode54
tools/picture_utils.h:54: SkPicture* resize_if_needed(SkPicture* picture,
SkAutoUnref* aur);
picture could be marked const

https://codereview.appspot.com/6495125/diff/1/tools/render_pictures_main.cpp
File tools/render_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/render_pictures_main.cpp#...
tools/render_pictures_main.cpp:104: SkPicture* picture = new
SkPicture(&inputStream);
SkNEW_ARGS

https://codereview.appspot.com/6495125/diff/1/tools/render_pictures_main.cpp#...
tools/render_pictures_main.cpp:107: SkDebugf("drawing... [%i %i] %s\n",
picture->width(), picture->height(),
Again, I think it would make sense to change the size before reporting it, so we
report the size we're actually drawing to.

[EricB, 2012-09-13 21:57:24]

Uploaded patch set 2.

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp
File tools/bench_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp#n...
tools/bench_pictures_main.cpp:112: SkPicture* picture = new
SkPicture(&inputStream);
On 2012/09/13 21:31:40, scroggo-work wrote:
> SkNEW_ARGS.

Done.

https://codereview.appspot.com/6495125/diff/1/tools/bench_pictures_main.cpp#n...
tools/bench_pictures_main.cpp:120: picture->height(), filename.c_str());
On 2012/09/13 21:31:40, scroggo-work wrote:
> Should we print this after doing the resize?

Since the resize function prints a note that it's resizing, I think it's useful
to know what the original was.

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp
File tools/picture_utils.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode117
tools/picture_utils.cpp:117: SkPicture* resize_if_needed(SkPicture* picture,
SkAutoUnref* aur) {
On 2012/09/13 21:31:40, scroggo-work wrote:
> Would it make more sense to take SkAutoTUnref<SkPicture>?
> 
> There may be good reasons to do it as is, but it feels odd to me that you are
> passing in an SkAutoUnref and the SkPicture it owns.
> 
> At the end of this function, aur points to the new picture, so you could even
> get away with returning void and calling aur to get to the picture.
> 
> Again, it just feels odd to me that there are multiple ways to get at the same
> data.

Yep. That makes more sense. Done.

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode121
tools/picture_utils.cpp:121: pic = new SkPicture();
On 2012/09/13 21:31:40, scroggo-work wrote:
> SkNEW

Done.

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode122
tools/picture_utils.cpp:122: picture->ref();
On 2012/09/13 21:31:40, scroggo-work wrote:
> Won't we need to unref this?

I think it gets unreffed in the call at line 129.  I had to add this to prevent
refCnt < 0.

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.h
File tools/picture_utils.h (right):

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.h#newcode53
tools/picture_utils.h:53: // and returns a new SkPicture.  Otherwise, the given
SkPicture is returned.
On 2012/09/13 21:31:40, scroggo-work wrote:
> Could you expand on this comment? At the end of this function, aur now points
to
> the new picture that is returned (if anything has changed).

Replaced to reflect the new parameters.

https://codereview.appspot.com/6495125/diff/1/tools/render_pictures_main.cpp
File tools/render_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/render_pictures_main.cpp#...
tools/render_pictures_main.cpp:104: SkPicture* picture = new
SkPicture(&inputStream);
On 2012/09/13 21:31:40, scroggo-work wrote:
> SkNEW_ARGS

Done.

[robertphillips, 2012-09-14 12:10:04]

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp
File tools/picture_utils.cpp (right):

https://codereview.appspot.com/6495125/diff/1/tools/picture_utils.cpp#newcode122
tools/picture_utils.cpp:122: picture->ref();
drawPicture takes a SkPicture& so it shouldn't be unreffing it

https://codereview.appspot.com/6495125/diff/3002/tools/picture_utils.cpp
File tools/picture_utils.cpp (right):

https://codereview.appspot.com/6495125/diff/3002/tools/picture_utils.cpp#newc...
tools/picture_utils.cpp:134: newSize.width(), newSize.height());
Don't we need to unref picture here?

https://codereview.appspot.com/6495125/diff/3002/tools/render_pictures_main.cpp
File tools/render_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/3002/tools/render_pictures_main.c...
tools/render_pictures_main.cpp:112: 
Doesn't this need to be aur?

[EricB, 2012-09-14 12:38:25]

Uploaded patch set 3.

https://codereview.appspot.com/6495125/diff/3002/tools/picture_utils.cpp
File tools/picture_utils.cpp (right):

https://codereview.appspot.com/6495125/diff/3002/tools/picture_utils.cpp#newc...
tools/picture_utils.cpp:134: newSize.width(), newSize.height());
On 2012/09/14 12:10:04, robertphillips wrote:
> Don't we need to unref picture here?

Done.

https://codereview.appspot.com/6495125/diff/3002/tools/render_pictures_main.cpp
File tools/render_pictures_main.cpp (right):

https://codereview.appspot.com/6495125/diff/3002/tools/render_pictures_main.c...
tools/render_pictures_main.cpp:112: 
On 2012/09/14 12:10:04, robertphillips wrote:
> Doesn't this need to be aur?

Done, here and in bench_pictures.

[robertphillips, 2012-09-14 12:40:02]

LGTM - but I defer to Leon

[Leon, 2012-09-14 14:29:22]

On 2012/09/14 12:40:02, robertphillips wrote:
> LGTM - but I defer to Leon

LGTM