Sky should use an SkPicture when drawing

Sky should use an SkPicture when drawing

This CL teaches the Sky compositor to record an SkPicture instead of just
drawing in immediate mode. Using an SkPicture will let us experiment with
various optimizations in Skia that preprocess the SkPicture before translating
the Skia commands to GL commands.

This CL also refactors the "display delegate" system now that we can hand off
an SkPicture to a rasterization backend. The new design requires fewer
backpointers and removes the static display delegate factory.

R=eseidel@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/20f9545776ad59ce5d7425f4fbbef9d2e4cd9b95

[eseidel1, 2015-01-20 02:04:03]

eseidel@google.com changed reviewers:
+ eseidel@google.com

[eseidel1, 2015-01-20 02:04:03]

lgtm

james is probably a better reviewer for some of this.

https://codereview.chromium.org/854303002/diff/1/sky/compositor/rasterizer_ga...
File sky/compositor/rasterizer_ganesh.cc (right):

https://codereview.chromium.org/854303002/diff/1/sky/compositor/rasterizer_ga...
sky/compositor/rasterizer_ganesh.cc:26: gfx::Size size(cull_rect.width(),
cull_rect.height());
gfx::rect doesn't have a .size()?

https://codereview.chromium.org/854303002/diff/1/sky/viewer/runtime_flags.h
File sky/viewer/runtime_flags.h (right):

https://codereview.chromium.org/854303002/diff/1/sky/viewer/runtime_flags.h#n...
sky/viewer/runtime_flags.h:14: struct RuntimeFlags {
No struct ever stays a struct.  I'd just make this a real class.  A shame that
c++ makes adding simple get/setters more difficult than it should.

https://codereview.chromium.org/854303002/diff/1/sky/viewer/runtime_flags.h#n...
sky/viewer/runtime_flags.h:15: bool testing;
Seems a bit strange, but OK.

[abarth-chromium, 2015-01-20 02:47:38]

https://codereview.chromium.org/854303002/diff/1/sky/compositor/rasterizer_ga...
File sky/compositor/rasterizer_ganesh.cc (right):

https://codereview.chromium.org/854303002/diff/1/sky/compositor/rasterizer_ga...
sky/compositor/rasterizer_ganesh.cc:26: gfx::Size size(cull_rect.width(),
cull_rect.height());
On 2015/01/20 at 02:04:03, eseidel1 wrote:
> gfx::rect doesn't have a .size()?

It's an SkRect.  I've added in the type.

https://codereview.chromium.org/854303002/diff/1/sky/viewer/runtime_flags.h
File sky/viewer/runtime_flags.h (right):

https://codereview.chromium.org/854303002/diff/1/sky/viewer/runtime_flags.h#n...
sky/viewer/runtime_flags.h:14: struct RuntimeFlags {
On 2015/01/20 at 02:04:03, eseidel1 wrote:
> No struct ever stays a struct.  I'd just make this a real class.  A shame that
c++ makes adding simple get/setters more difficult than it should.

Fixed.

[abarth-chromium, 2015-01-20 02:47:57]

Committed patchset #2 (id:20001) manually as
20f9545776ad59ce5d7425f4fbbef9d2e4cd9b95 (presubmit successful).

[jamesr, 2015-01-22 01:20:44]

jamesr@chromium.org changed reviewers:
+ jamesr@chromium.org

[jamesr, 2015-01-22 01:20:45]

https://codereview.chromium.org/854303002/diff/20001/sky/viewer/document_view.cc
File sky/viewer/document_view.cc (right):

https://codereview.chromium.org/854303002/diff/20001/sky/viewer/document_view...
sky/viewer/document_view.cc:159: DCHECK(RuntimeFlags::Get().testing()) <<
"Requires testing runtime flag";
fyi this check fires when loading up most of the sky tests in skydb

[abarth-chromium, 2015-01-22 02:05:33]

https://codereview.chromium.org/854303002/diff/20001/sky/viewer/document_view.cc
File sky/viewer/document_view.cc (right):

https://codereview.chromium.org/854303002/diff/20001/sky/viewer/document_view...
sky/viewer/document_view.cc:159: DCHECK(RuntimeFlags::Get().testing()) <<
"Requires testing runtime flag";
On 2015/01/22 at 01:20:44, jamesr wrote:
> fyi this check fires when loading up most of the sky tests in skydb

Interesting.  We should catch that in internals.cc instead of here.

[abarth-chromium, 2015-01-22 02:18:43]

Fix in https://codereview.chromium.org/866653002