Update the UI examples.

Update the UI examples.

Port the Spinning Cube, Tile, Ganesh (aka. Shapes), PNG, and PDF examples
to the new Mozart compositor and view manager.

BUG=
R=abarth@google.com, viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/43ebadc2a0d68487739cecbbd3393f852b7d3d7e

[jeffbrown, 2015-12-31 11:20:38]

jeffbrown@google.com changed reviewers:
+ abarth@google.com, jamesr@chromium.org, viettrungluu@chromium.org

[abarth, 2016-01-10 23:09:32]

LGTM

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
File examples/ui/pdf_viewer/pdf_viewer.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
examples/ui/pdf_viewer/pdf_viewer.cc:128: UpdateScene();
I'm surprised there's no token that I get from layout_params that I pass through
when publishing my scene.  I was expecting to get some sort of version number or
something so that my parent and I can coordinate resizes.  Maybe I forgot how
that works.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
examples/ui/pdf_viewer/pdf_viewer.cc:227: UpdateScene();
I was expecting some sort of "invalidate and wait for the choreographer" pattern
here.  If I spam the keyboard, does this mean a re-raster the PDF multiple times
per vsync?

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
File examples/ui/spinning_cube/spinning_cube_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:161:
root_node->content_transform->matrix[5] = -1;  // flip image vertically
Why?

[viettrungluu, 2016-01-13 00:10:57]

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
File examples/ui/shapes/shapes_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
examples/ui/shapes/shapes_view.cc:45: auto update =
mojo::gfx::composition::SceneUpdate::New();
Since this is a basic example, a few more (brief) comments might be nice.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
File examples/ui/shapes/shapes_view.h (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
examples/ui/shapes/shapes_view.h:23: // |View|:
This comment is accurate, though it was a bit confusing to me since this class
doesn't directly derive from View (indeed, this is a few steps up the class
hierarchy).

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
File examples/ui/spinning_cube/spinning_cube_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:30: return hypot(start.x -
end.x, start.y - end.y);
- include <math.h> for this
- possibly, you may want to use hypotf() instead of hypot() [*]

[*] C++11 specifies that <cmath> has an overloaded std::hypot(). Thus C++11's
<math.h> is supposed to get an overloaded ::hypot() as well. This is quite
confusing, since obviously C's <math.h> doesn't have the overload. So maybe you
should include <cmath> and write it as std::hypot(). Blech.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:44: std::abs(current.y -
initial.y) < std::abs(current.x - initial.x);
To get the std::abs overloads for float or double, you should include <cmath>.
(Note that <cstdlib> has the overloads for integral types.)

(Theoretically, comments about <math.h>/<stdlib.h> apply as above. Note that
AFAIK these are not *required* by the standard to define the std:: versions.)

Though arguably it'd just be clearer to use the un-overloaded <math.h> fabsf()
(or fabs())....

[viettrungluu, 2016-01-13 00:12:18]

On 2016/01/13 00:10:57, viettrungluu wrote:
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
> File examples/ui/shapes/shapes_view.cc (right):
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
> examples/ui/shapes/shapes_view.cc:45: auto update =
> mojo::gfx::composition::SceneUpdate::New();
> Since this is a basic example, a few more (brief) comments might be nice.
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
> File examples/ui/shapes/shapes_view.h (right):
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
> examples/ui/shapes/shapes_view.h:23: // |View|:
> This comment is accurate, though it was a bit confusing to me since this class
> doesn't directly derive from View (indeed, this is a few steps up the class
> hierarchy).
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
> File examples/ui/spinning_cube/spinning_cube_view.cc (right):
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
> examples/ui/spinning_cube/spinning_cube_view.cc:30: return hypot(start.x -
> end.x, start.y - end.y);
> - include <math.h> for this
> - possibly, you may want to use hypotf() instead of hypot() [*]
> 
> [*] C++11 specifies that <cmath> has an overloaded std::hypot(). Thus C++11's
> <math.h> is supposed to get an overloaded ::hypot() as well. This is quite
> confusing, since obviously C's <math.h> doesn't have the overload. So maybe
you
> should include <cmath> and write it as std::hypot(). Blech.
> 
>
https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
> examples/ui/spinning_cube/spinning_cube_view.cc:44: std::abs(current.y -
> initial.y) < std::abs(current.x - initial.x);
> To get the std::abs overloads for float or double, you should include <cmath>.
> (Note that <cstdlib> has the overloads for integral types.)
> 
> (Theoretically, comments about <math.h>/<stdlib.h> apply as above. Note that
> AFAIK these are not *required* by the standard to define the std:: versions.)
> 
> Though arguably it'd just be clearer to use the un-overloaded <math.h> fabsf()
> (or fabs())....

(Also, apart from various nits, LGTM too, at least for the parts I read.)

[jeffbrown, 2016-01-26 08:14:17]

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
File examples/ui/pdf_viewer/pdf_viewer.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
examples/ui/pdf_viewer/pdf_viewer.cc:128: UpdateScene();
On 2016/01/10 23:09:31, abarth wrote:
> I'm surprised there's no token that I get from layout_params that I pass
through
> when publishing my scene.  I was expecting to get some sort of version number
or
> something so that my parent and I can coordinate resizes.  Maybe I forgot how
> that works.

I'll be redoing layout once this gets checked in.  And as you expected, there
will be a version passed in somewhere.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/pdf_viewer/pdf_...
examples/ui/pdf_viewer/pdf_viewer.cc:227: UpdateScene();
On 2016/01/10 23:09:31, abarth wrote:
> I was expecting some sort of "invalidate and wait for the choreographer"
pattern
> here.  If I spam the keyboard, does this mean a re-raster the PDF multiple
times
> per vsync?

Good point.  Let's add that.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
File examples/ui/shapes/shapes_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
examples/ui/shapes/shapes_view.cc:45: auto update =
mojo::gfx::composition::SceneUpdate::New();
On 2016/01/13 00:10:57, viettrungluu wrote:
> Since this is a basic example, a few more (brief) comments might be nice.

Done.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
File examples/ui/shapes/shapes_view.h (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/shapes/shapes_v...
examples/ui/shapes/shapes_view.h:23: // |View|:
On 2016/01/13 00:10:57, viettrungluu wrote:
> This comment is accurate, though it was a bit confusing to me since this class
> doesn't directly derive from View (indeed, this is a few steps up the class
> hierarchy).

Good point.  I'll change these.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
File examples/ui/spinning_cube/spinning_cube_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:30: return hypot(start.x -
end.x, start.y - end.y);
On 2016/01/13 00:10:57, viettrungluu wrote:
> - include <math.h> for this
> - possibly, you may want to use hypotf() instead of hypot() [*]
> 
> [*] C++11 specifies that <cmath> has an overloaded std::hypot(). Thus C++11's
> <math.h> is supposed to get an overloaded ::hypot() as well. This is quite
> confusing, since obviously C's <math.h> doesn't have the overload. So maybe
you
> should include <cmath> and write it as std::hypot(). Blech.

Let's use <cmath>.  Note that this code was just copied straight out of the old
spinning cube.

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:44: std::abs(current.y -
initial.y) < std::abs(current.x - initial.x);
On 2016/01/13 00:10:57, viettrungluu wrote:
> To get the std::abs overloads for float or double, you should include <cmath>.
> (Note that <cstdlib> has the overloads for integral types.)
> 
> (Theoretically, comments about <math.h>/<stdlib.h> apply as above. Note that
> AFAIK these are not *required* by the standard to define the std:: versions.)
> 
> Though arguably it'd just be clearer to use the un-overloaded <math.h> fabsf()
> (or fabs())....

Since we want the float version I just included cmath.  Ahh C++.

[jeffbrown, 2016-01-26 08:23:06]

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
File examples/ui/spinning_cube/spinning_cube_view.cc (right):

https://codereview.chromium.org/1559723002/diff/1/examples/ui/spinning_cube/s...
examples/ui/spinning_cube/spinning_cube_view.cc:161:
root_node->content_transform->matrix[5] = -1;  // flip image vertically
On 2016/01/10 23:09:31, abarth wrote:
> Why?

I'm not sure exactly.  The coordinate system ends up being inverted.  I actually
have the feeling that Spinning Cube itself is using a weird view matrix since
other GL content is composited in the correct orientation.  I'll leave a TODO
since this is kind of silly.

[jeffbrown, 2016-01-26 09:23:47]

address feedback

[jeffbrown, 2016-01-26 23:52:09]

Description was changed from

==========
Update the UI examples.

Port the Spinning Cube, Tile, Ganesh (aka. Shapes), PNG, and PDF examples
to the new Mozart compositor and view manager.

BUG=
==========

to

==========
Update the UI examples.

Port the Spinning Cube, Tile, Ganesh (aka. Shapes), PNG, and PDF examples
to the new Mozart compositor and view manager.

BUG=
R=abarth@google.com, viettrungluu@chromium.org

Committed:
https://chromium.googlesource.com/external/mojo/+/43ebadc2a0d68487739cecbbd33...
==========

[jeffbrown, 2016-01-26 23:52:10]

Committed patchset #2 (id:20001) manually as
43ebadc2a0d68487739cecbbd3393f852b7d3d7e (presubmit successful).