Try out scalar picture sizes

Try out scalar picture sizes

This paves the way for removing the 'fTile' parameter from SkPictureShader (although that should be a different CL). If we like this we could also move to providing an entire cull SkRect.

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

[robertphillips, 2014-08-28 14:31:07]

robertphillips@google.com changed reviewers:
+ fmalita@chromium.org, fmalita@google.com, mtklein@google.com, reed@google.com

[reed1, 2014-08-28 14:36:48]

I wonder, just for staging, if you'll need to have

#ifdef SK_SUPPORT_LEGACY_INT_PICT_WIDTH
int width() const;
#endif

SkScalar cullWidth() const;

In case chrome has places that will fail or generate warnings with the type
change from int to float

Other than that, I like the api in picture.

[f(malita), 2014-08-28 15:02:58]

On 2014/08/28 14:36:48, reed1 wrote:
> I wonder, just for staging, if you'll need to have
> 
> #ifdef SK_SUPPORT_LEGACY_INT_PICT_WIDTH
> int width() const;
> #endif
> 
> SkScalar cullWidth() const;
> 
> In case chrome has places that will fail or generate warnings with the type
> change from int to float

picture.cc:
  gfx::Rect layer_rect(skpicture->width(), skpicture->height());
  gfx::Rect opaque_rect(skpicture->width(), skpicture->height());

(IIRC gfx::Rect is their RectI).

pixel_ref_utils.cc:

  empty_bitmap.setInfo(SkImageInfo::MakeUnknown(picture->width(),
picture->height()));

(possibly other spots not indexed in my IDE.)


> Other than that, I like the api in picture.

+1

[robertphillips, 2014-08-28 20:47:17]

PTAL this adds Chromium & Android staging and removes Skia's usage of the
SkPicture width() & height() entry points.

[reed1, 2014-08-28 20:54:18]

lgtm

https://codereview.chromium.org/513983002/diff/120001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/513983002/diff/120001/include/core/SkPicture....
include/core/SkPicture.h:180: static bool
InternalOnly_BufferIsSKP(SkReadBuffer*, SkPictInfo*);
funny that this change is part of this CL

[robertphillips, 2014-08-28 21:02:00]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-08-28 21:02:51]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/513983002/1...

[commit-bot: I haz the power, 2014-08-28 21:03:06]

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

[commit-bot: I haz the power, 2014-08-28 21:03:07]

Failed to apply patch for gyp/skia_for_chromium_defines.gypi:
While running git apply --index -p1;
  error: patch failed: gyp/skia_for_chromium_defines.gypi:18
  error: gyp/skia_for_chromium_defines.gypi: patch does not apply

Patch:       gyp/skia_for_chromium_defines.gypi
Index: gyp/skia_for_chromium_defines.gypi
diff --git a/gyp/skia_for_chromium_defines.gypi
b/gyp/skia_for_chromium_defines.gypi
index
53fc7b9c6308ca438dd11cc3938fda4c314787e1..8bc98ed567f02a4e2b50a7fe0ab68fef46d72d8f
100644
--- a/gyp/skia_for_chromium_defines.gypi
+++ b/gyp/skia_for_chromium_defines.gypi
@@ -18,6 +18,7 @@
       'SK_SUPPORT_LEGACY_GETDEVICE',
       'SK_IGNORE_ETC1_SUPPORT',
       'SK_IGNORE_GPU_DITHER',
+      'SK_LEGACY_PICTURE_SIZE_API',
     ],
   },
 }

[robertphillips, 2014-08-28 21:51:59]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-08-28 21:52:15]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/513983002/1...

[commit-bot: I haz the power, 2014-08-28 22:09:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Mac10.8-Clang-x86_64-Release-Trybo...)
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-Clang-x86_64-Debug-Try...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-GCC4.8-x86_64-Release-...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Relea...)

[commit-bot: I haz the power, 2014-08-28 22:11:44]

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

[commit-bot: I haz the power, 2014-08-28 22:11:45]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug...)

[robertphillips, 2014-08-29 14:36:37]

The CQ bit was checked by robertphillips@google.com

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

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/513983002/1...

[commit-bot: I haz the power, 2014-08-29 15:04:03]

Committed patchset #9 (id:160001) as a8d7f0b13cd4c6d773fcf055fe17db75d260fa05