Zoom handler doesn't interact well with PSwing nodes

[mro]

Originally reported on Google Code with ID 154

See comments on Issue 15:

http://code.google.com/p/piccolo2d/issues/detail?id=15#c5
http://code.google.com/p/piccolo2d/issues/detail?id=15#c6

Reported by heuermh on 2009-12-18 17:38:44

[mro]

Reported by heuermh on 2010-08-27 16:17:52

• Labels added: Toolkit-Piccolo2D.Java

[mro]

Let's try to get this into 1.3.1.

Reported by heuermh on 2010-12-22 01:59:50

• Status changed: Accepted
• Labels added: Type-Defect, Priority-Medium, Effort-Medium, OpSys-All, Component-Extras, Milestone-1.3.1

[mro]

This logic was introduced by Issue 15:

PPickPath::processEvent()

343 | if (event.isHandled()) {
344 |    return;
345 | }

But I'm not sure why this "short-circuiting" is required. When I remove it, I don't
seem to be able to reproduce the behavior described in Issue 15; that is, I don't see
events sent to overlapped nodes.

(1) Am I missing something?

But this short-circuiting does stop event handling up the pick path (so that ancestor
node listeners won't get signalled). Is this really the desired behavior? When we short-circuit,
the canvas-level handlers (pan, zoom) now won't get the event.

Without the above logic, event handlers can still do their own checking of event.isHandled()
and decide not to handle the events. In addition, there's PEventFilter.setAcceptsAlreadyHandledEvents()
that handlers can use.

(2) Does it make sense to remove this short-circuiting behavior in Piccolo 2.0?

We can't remove it in 1.4 point release for compatiblity but we could, in 1.4, make
it a configurable capability to disable short-cutting, I suppose at the camera or canvas
level, something like:

> camera.setPropagatesAlreadyHandledEvents()

This would be false by default (backwards-compatible behavior) but could be set to
true by clients wanting to avoid the bad zoom handling behavior. They'd have to do
something like this to get the good zoom behavior:

camera.setPropagatesAlreadyHandledEvents(true);
// even w/o the short-circuting, by default the zoom handlers won't respond to already
handled events,
// so we'd have to do something like this
PInputEventFilter ef = new PInputEventFilter(mask);
ef.setAcceptsAlreadyHandledEvents(true);
canvas.getZoomEventHandler().setEventFilter(ef);

(3) Thoughts?

Reported by atdixon on 2011-03-05 20:27:47

[mro]

In the interest of a 1.3.1 release, I propose we move this to 1.4 milestone so we can
give time for discussion of previous comment.

I'll wait for votes (or absence of votes), and then move to a 1.4.

Reported by atdixon on 2011-03-20 21:10:31

[mro]

In the interest of a 1.3.1 release, I propose we move this to 1.4 milestone so we can
give time for discussion of previous comment.

I'll wait for votes (or absence of votes), and then move to a 1.4.

Reported by atdixon on 2011-03-20 21:10:32

[mro]

Moving to 1.4 as further discussion is needed.

Reported by atdixon on 2011-03-27 02:17:51

• Labels added: Milestone-1.4
• Labels removed: Milestone-1.3.1