google-code-export / piccolo2d

Automatically exported from code.google.com/p/piccolo2d
2 stars 1 forks source link

Zoom handler doesn't interact well with PSwing nodes #154

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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

Original issue reported on code.google.com by heue...@gmail.com on 18 Dec 2009 at 5:38

GoogleCodeExporter commented 9 years ago

Original comment by heue...@gmail.com on 27 Aug 2010 at 4:17

GoogleCodeExporter commented 9 years ago
Let's try to get this into 1.3.1.

Original comment by heue...@gmail.com on 22 Dec 2010 at 1:59

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
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?

Original comment by atdi...@gmail.com on 5 Mar 2011 at 8:27

GoogleCodeExporter commented 9 years ago
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.

Original comment by atdi...@gmail.com on 20 Mar 2011 at 9:10

GoogleCodeExporter commented 9 years ago
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.

Original comment by atdi...@gmail.com on 20 Mar 2011 at 9:10

GoogleCodeExporter commented 9 years ago
Moving to 1.4 as further discussion is needed.

Original comment by atdi...@gmail.com on 27 Mar 2011 at 2:17