Mozart: Implement basic input event dispatch with hit testing.

Mozart: Implement basic input event dispatch with hit testing.

Whenever a POINTER_DOWN occurs, finds the first view which
was hit and directs all subsequent events over there.

This policy is obviously broken in many ways but will suffice
for scaffolding of other elements.  We will overhaul the input
event protocol entirely in future patches.

Updated the examples to include a minimal hit test behavior
specification so they can continue to receive input events in the
ways they previously did.

BUG=
R=abarth@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/ca132f35854a195bcab106f8fd6ba70a36ad68b5

[jeffbrown, 2016-03-09 02:23:38]

jeffbrown@google.com changed reviewers:
+ abarth@google.com

[abarth, 2016-03-09 04:17:37]

lgtm

https://codereview.chromium.org/1776473005/diff/1/mojo/services/geometry/cpp/...
File mojo/services/geometry/cpp/geometry_util.cc (right):

https://codereview.chromium.org/1776473005/diff/1/mojo/services/geometry/cpp/...
mojo/services/geometry/cpp/geometry_util.cc:35: float z = transform.matrix[12] *
point.x + transform.matrix[13] * point.y +
We usually call this quantity `w`

https://codereview.chromium.org/1776473005/diff/1/mojo/services/geometry/cpp/...
mojo/services/geometry/cpp/geometry_util.cc:38: z = 1.f / z;
If `w` is zero, the Point should be at infinity.  Given that it's a int32, we
should probably use MAX_INT.

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
File services/ui/input_manager/input_associate.cc (right):

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_associate.cc:82: new InputConnectionImpl(this,
view_token.Pass(), request.Pass())));
With emplace, I would have thought you could skip the std::unique_ptr part.

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
File services/ui/input_manager/input_dispatcher_impl.cc (right):

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_dispatcher_impl.cc:91:
weak_factory_.GetWeakPtr()));
Interesting.

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_dispatcher_impl.cc:119: scene =
scene->hits[0]->get_scene().get();
I see, you're just delivering to the most specific hit.  That seems fine for
now.

[jeffbrown, 2016-03-09 19:38:50]

https://codereview.chromium.org/1776473005/diff/1/mojo/services/geometry/cpp/...
File mojo/services/geometry/cpp/geometry_util.cc (right):

https://codereview.chromium.org/1776473005/diff/1/mojo/services/geometry/cpp/...
mojo/services/geometry/cpp/geometry_util.cc:38: z = 1.f / z;
On 2016/03/09 04:17:37, abarth wrote:
> If `w` is zero, the Point should be at infinity.  Given that it's a int32, we
> should probably use MAX_INT.

Well, but which infinity?  This behavior is consistent with Skia's
interpretation.  I'll leave a TODO to revisit.

Actually that points out another problem with potential loss of precision in
this code due to the use of integer points.  Need to think about whether that's
the right thing for the compositor to be dealing with.  I probably want to be
using PointF and RectF.

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
File services/ui/input_manager/input_associate.cc (right):

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_associate.cc:82: new InputConnectionImpl(this,
view_token.Pass(), request.Pass())));
On 2016/03/09 04:17:37, abarth wrote:
> With emplace, I would have thought you could skip the std::unique_ptr part.

Me too but it turns out that what's being emplaced is an std::pair<> and the
compiler will not implicitly convert the bare pointer to std::unique_ptr<> so it
fails to resolve this.  Wishing we had make_unique someplace actually useful in
the tree...

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
File services/ui/input_manager/input_dispatcher_impl.cc (right):

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_dispatcher_impl.cc:91:
weak_factory_.GetWeakPtr()));
On 2016/03/09 04:17:37, abarth wrote:
> Interesting.

This was to prevent reentrance when called from OnHitTestResult since the
callback may be invoked immediately.  Usually I would ensure that the callback
runs on the message loop to avoid just such a situation.  I think I can clean
this up another way though.

https://codereview.chromium.org/1776473005/diff/1/services/ui/input_manager/i...
services/ui/input_manager/input_dispatcher_impl.cc:119: scene =
scene->hits[0]->get_scene().get();
On 2016/03/09 04:17:37, abarth wrote:
> I see, you're just delivering to the most specific hit.  That seems fine for
> now.

Acknowledged.

[jeffbrown, 2016-03-09 22:23:41]

Description was changed from

==========
Mozart: Implement basic input event dispatch with hit testing.

Whenever a POINTER_DOWN occurs, finds the first view which
was hit and directs all subsequent events over there.

This policy is obviously broken in many ways but will suffice
for scaffolding of other elements.  We will overhaul the input
event protocol entirely in future patches.

Updated the examples to include a minimal hit test behavior
specification so they can continue to receive input events in the
ways they previously did.

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

to

==========
Mozart: Implement basic input event dispatch with hit testing.

Whenever a POINTER_DOWN occurs, finds the first view which
was hit and directs all subsequent events over there.

This policy is obviously broken in many ways but will suffice
for scaffolding of other elements.  We will overhaul the input
event protocol entirely in future patches.

Updated the examples to include a minimal hit test behavior
specification so they can continue to receive input events in the
ways they previously did.

BUG=
R=abarth@google.com

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

[jeffbrown, 2016-03-09 22:23:43]

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