Should nodes have pickable:false by default?

[samreid]

To optimize the performance of a sim, one of the steps is to visit all of the non-interactive nodes and set pickable:false. Would it be better for pickable:false to be the default, and when an input listener is added pickable can automatically switch to true?

[pixelzoom]

Would it be better for pickable:false to be the default, and when an input listener is added pickable can automatically switch to true?

Sound like a nice convenience, but...

I frequently have a scenery.Node that is used as the parent for building a more complicated interactive object. I want the entire object to be interactive, so I add my InputListener to the parent Node. So which node(s) are then automatically switched to pickable:true? If it's just the parent Node, then the children are still pickable:false and I have nothing to interact with. If all children also automatically get pickable:true, then there will be an ordering dependency; I'll have to add children before adding the InputListener. Or something an additional check will need to be introduced in addChild to see if any ancestor has an InputListener. Something similarly messy for removeChild.

[samreid]

@pixelzoom makes a good point. I'm happy to close this ticket, but perhaps I'll assign it to @jonathanolson to have him veriew and verify first.

[jonathanolson]

When this issue was created, I would imagine that adding in an event listener on a node would make that node and all descendants implicitly pickable, but a node that doesn't have an input event listener in its trail (to the scene root) would be by default unpickable.

[pixelzoom]

Well, that's going to introduce an order dependency then. If you have any descendants that shouldn't be pickable, you'll need to set their pickable:false after calling addInputListener. And what happens on removeInputListener? Does the node and all of its descendents revert to pickable:false? Smells like trouble.

[jonathanolson]

It would just mean we would need to introduce a default of something like pickable: undefined (meaning it is pickable if there is a node in its trail with an input listener), pickable: false and pickable: true would still behave as expected.

Note that undefined/false/true may not be the best choice, we could go with an enumeration pattern, etc.

[samreid]

Pickable: undefined sounds like it might work well (though it would probably need a better name). I'd be interested to hear @pixelzoom thoughts about that, or if there are any anticipated problems with that. But I should say, having to manually specify 'pickable:false' in my non-pickable nodes doesn't really slow me down too much.

[pixelzoom]

I think we should leave things as is (default to pickable:false). We have bigger fish to fry, and I don't see this as significant overhead.

[jonathanolson]

An alternate proposal:

• pickable: undefined - default. Node is only pickable if it (or an ancestor) has an input listener attached, or pickable: true set.
• pickable: true - Node is pickable
• pickable: false - Node is unpickable (has an effect only when underneath a node with an input listener / pickable: true

This means the default is "pass-through" for all nodes without listeners attached. If you want a node to "block" events, you need to explicitly set pickable: true. The hit testing performance would be optimized with this, and would reduce sim 'picakble' flags.

It would come at the cost of slightly increased Scenery complexity, but I believe the performance benefits would be overall a net plus.

[jonathanolson]

pickable: null (instead of undefined)

[pixelzoom]

10/3/13 dev meeting consensus: @jonathanolson proposal sounds good, use null instead of undefined.

[jonathanolson]

From the documentation (for precision):

1. If the node or one of its ancestors has pickable: false OR is invisible, the node will not receive events or hit testing
2. If the node or one of its ancestors or descendants is pickable: true OR has an input listener attached, it will receive events or hit testing
3. Otherwise, it will not receive events or hit testing