The electron shells in Build an Atom are depicted as dotted lines. This works fine on Windows + Chrome, but these lines appear solid on an iPad 2.

Reported by Noah, probably each resize event rebuilds everything, have layers be persistent.

Should be handled with #30

Fields transform, children and parents shouldn't be modified directly. Their names should have an "_", the convention used in scenery to indicate "private".

Running FAMB/master on iOS/Safari, when attaching the 2nd puller (or more) to the rope, all pullers temporarily disappear then reappear. Perhaps something to do with the "go" button which appears after the first puller is attached? This is a new issue after scenery merged the instances-stitch branch into master.

In beers-law-lab, the Concentration meter (ConcentrationMeterNode) is composed of 3 images (left, center and right) that look like this:

For i18n, the center node is repeated to make the Concentration as wide as necessary. This is done in HorizontalTiledNode, where copies of the center image are made using "new Image(centerNode.getImage())".

In the composite node rendered by Scenery, the center images are darker than the left and right. Here's a screenshot:

Here's a Gimp screenshot that shows that the images all have the same color:

Is Image.getImage() creating an image that is not true to the original?

This is a topic for discussion.

Like Piccolo, Scenery includes invisible children in bounds computations for the parent node. There are some use cases where this is inconvenient, for example layout involving a node that has components that are selectively hidden. This can be worked around by using addChild/removeChild instead of the visible property, but maintain rendering order becomes a little messy.

I don't have a strong feeling either way on this. But because this was somewhat of an issue with Piccolo, I'd like us to make a conscious decision after looking at pros and cons.

If I start FAMB on Win/Chrome, then resize the window and drop a puller on the rope, I get this exception:

Uncaught Error: Assertion failed: undefined assert.js:39
(anonymous function) assert.js:39
Layer.removeNodeFromTrail Layer.js:194
_.extend.removeNodeFromTrail CanvasLayer.js:377
(anonymous function) Scene.js:116
TrailPointer.eachTrailBetween TrailPointer.js:211
Trail.eachTrailUnder Trail.js:303
Scene.sceneEventListener.markForRemoval Scene.js:114
(anonymous function) Node.js:599
_.each._.forEach underscore-1.4.2.js:79
Node.fireEvent Node.js:597
recursiveEventDispatch Node.js:626
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
(anonymous function) Node.js:629
_.each._.forEach underscore-1.4.2.js:79
recursiveEventDispatch Node.js:628
Node.dispatchEvent Node.js:635
Node.removeChild Node.js:165
(anonymous function) Node.js:189
_.each._.forEach underscore-1.4.2.js:79
Node.setChildren Node.js:188
children Node.js:1327
(anonymous function) GoButton.js:45
triggerEvents backbone.js:118
Backbone.Events.trigger backbone.js:220
_.extend.set backbone.js:401
Object.defineProperty.set Fort.js:121
(anonymous function) TugOfWarModel.js:91
triggerEvents backbone.js:118
Backbone.Events.trigger backbone.js:220
_.extend.set backbone.js:401
Object.defineProperty.set Fort.js:121
PullerNode.pullerNode.addInputListener.SimpleDragHandler.end PullerNode.js:63
SimpleDragHandler.endDrag SimpleDragHandler.js:143
dragListener.up SimpleDragHandler.js:69
Input.dispatchToPointer Input.js:354
Input.dispatchEvent Input.js:322
Input.upEvent Input.js:239
Input.mouseUp

Same exception does not occur if I don't resize the window after the sim is loaded. Any ideas?

Example: Run beers-law-lab.html. Switch between the tabs a couple of times. "Saturated!" will appear in the beaker of the "Concentration" tab, even though the solution is not saturated. The implementation is SaturatedIndicatorNode, and setting a breakpoint in this node's property listener confirms that visible is not being set to true. Also note that this node is consists of 2 nodes, Text and Rectangle, and only the Text node is becoming visible. So I suspect it has something to do with DOM.

I believe the SR has an example of this, too.

To determine the exact bounds of a Path with a stroke, Scenery needs to actually build a stroked version of the path (in Kite), and use those bounds.

Dragging the BLL meter on the iPad was significantly slowed down by Kite having to stroke a path and determine the stroked bounds every move event (even more than every frame!)

This generally should be unnecessary. It's possible to quickly compute a high-bound for a shape's stroked bounds given the line width and miter limit which could be used for Canvas dirty regions, or we could even skip calculation of stroked bounds (and disable Canvas dirty regions similar to Text).

Thus it seems like Path could benefit from a boundsMethod flag as well, although the options might be different (maybe they could be the same?)

in branch accessibility-3, tab focusing an item then resizing the window updates the focus region, but only if the window is getting larger. If the window is getting smaller, the focus region moves to the wrong spot.

I am using scenery with Forces and Motion Basics https://github.com/phetsims/forces-and-motion-basics master and having the following problem on iPad3 (works fine elsewhere). On the "motion" tab, if you drag a crate above the ground and let go, it nicely and smoothly animates to the top of the skateboard. However, if you drag either human above the ground, they doesn't animate at all. (Unless you drag your finger about the background after dropping them)

Switching ItemNode to render as canvas instead of svg solves that problem but really harms the performance.

All images are rendered with png, but one crucial difference between the humans and other objects is that upon release, the humans change to a 'sitting down' image. This could be related to (causing?) the problem.

Available for anyone who wants it.

Currently slower than it needs to be, and affects SVG / DOM performance.

Matrix3.getCSSTransformStyles() currently returns an object with a property for every CSS property that is needed (including prefixed ones). Using jQuery to apply all of these is not the fastest way.

Ideally we should auto-detect which prefixes are necessary (only set a few) if that helps with performance, and we should directly set the CSS styles with statements like element.style.transform = ...

Low-priority. If you do anything to a DOM node that changes its bounds, you need to explicitly call DOM.invalidateDOM. It would be nice if this could be done automatically. But no idea how to detect this.

scenery.Node.js is currently using

while ( node !== null ){}

to follow the parent chain. However, the parent of the root is undefined, not root, so this fails with an error. I'll commit a fix shortly.

JO and I discussed over Skype, so he is already aware of this. Call stack is shown below:

Uncaught Error: Assertion failed: Bounds must be empty or finite in invalidateSelf assert.js:39
(anonymous function) assert.js:39
Node.invalidateSelf Node.js:363
inherit.invalidateText Text.js:76
inherit.setText Text.js:64
updateElementName AtomView.js:61
AtomView AtomView.js:69
triggerEvents backbone.js:206
...etc...

What do you think about making Node.addInputListener and similar methods chainable by adding 'return this' at the bottom?

JO and I looked at this in a Skype call, creating a ticket here.

It looks like SimpleDragHandler is setting trail incorrectly for 'drag'. Here are the relevant lines:

37 this.trail = null; // stores the path to the node that is being dragged
124 this.trail = event.trail.subtrailTo( event.currentTarget, true );

This manifested itself in a drag handler I was writing in beers-law-lab. In this example, I would expect event.currentTarget in 'start' be the same node as trail.lastNode in 'drag'. They are different, and trail.lastNode is likely the parent node.

Here's the beers-law-lab code snippet, from ConcentrationSliderNode:

   * Drag handler for the slider thumb.
   * @param {Property} concentration of type number
   * @param {LinearFunction} positionToValue
   * @constructor
   */
  function ThumbDragHandler( concentration, positionToValue ) {
    var clickXOffset; // x-offset between initial click and thumb's origin
    var currentTarget; // save event.currentTarget for comparison with trail.lastNode
    SimpleDragHandler.call( this, {
      start: function ( event ) {
        currentTarget = event.currentTarget;
        clickXOffset = event.currentTarget.globalToParentPoint( event.pointer.point ).x - event.currentTarget.x;
      },
      drag: function ( event, trail ) {
        console.log( "expected node? " + ( currentTarget === trail.lastNode() ) );//XXX
        var x = trail.lastNode().globalToParentPoint( event.pointer.point ).x - clickXOffset;
        concentration.set( positionToValue.evaluate( x ) );
      },
      translate: function () {
        // do nothing, override default behavior
      }
    } );
  }

  inherit( ThumbDragHandler, SimpleDragHandler );

In beers-law-lab, ConcentrationMeterNode.WireNode is not updating when using the svg renderer.

A recent change to scenery causes this exception if I create a path with new Shape():
Uncaught Error: Assertion failed: Infinite bounds given to invalidateSelf

Should I be able to create a Shape/Path as an empty shape with new Shape()?

This is a feature we talked about in 5/9/13 developer meeting. This is a version of getBounds where only the visible children contribute to the bounds. It should also do something sensible when called on an invisible node.

If it is easy to implement, it might be a nice feature (and helpful) to animate the focus rectangle smoothly when focus target changes.

We should add support for setting/changing the focus order for tab traversal. Right now the order is equivalent to the order added to the scenery.Scene, which is not often accurate. The simplest thing to do first may be to give nodes the ability to set the tabindex for their peers directly. Later we should investigate support for a higher-level support that makes it easy to set tab groups, (such as being able to tab through all elements in a control panel instead of jumping around).

Available to anyone.

Setting the font parameters for SVG text is used both in actual updates AND for the fast bounds. I want to have something like:
element.setAttribute( 'font', this._font.getFont() );
Since getFont() is a simple getter (extremely fast) and setting one CSS-style attribute should be much faster. This wasn't working (I haven't investigated yet), but setting all of the attributes separately was working:
element.setAttribute( 'font-family', this._font.getFamily() );
element.setAttribute( 'font-size', this._font.getSize() );
element.setAttribute( 'font-style', this._font.getStyle() );
element.setAttribute( 'font-weight', this._font.getWeight() );
if ( this._font.getStretch() ) {
element.setAttribute( 'font-stretch', this._font.getStretch() );
}
This involves many more DOM manipulations to read out the font parameters (if necessary, it could be sped up), but also sets more attributes, and is significantly slower.

This ticket is a topic for discussion.

This is an area where Scenery deviates from Piccolo. In Piccolo, attempting to add a child that is already a child will move the child to the front. Attempting to remove a child that is not a child is a silent no-op. In scenery, both of these situations result in assertion failure.

In scenery, the most compelling reason for addChild failing is that scenery is a DAG. Scenery prohibits a node from being its own sibiling (not a general constraint of DAGs) because being its own sibling would mean rendered over itself, and it allowed JO to make a number of performance optimizations.

I saw this code in SliderNode.js (in beers-law-lab)

    // enable/disable knob
    enabled.addObserver( function ( enabled ) {
      knobNode.fill = enabled ? KNOB_FILL_ENABLED : KNOB_FILL_DISABLED;
      knobNode.cursor = enabled ? "pointer" : "default";
    } );

and it made me think we should discuss adding support for enabled/disabled directly into scenery. When disabled, a component should have cursor: default and not process any mouse or key events. It should also not participate in the focus cycle. By adding enable/disable support to scenery, we may avoid duplication (and divergence) of enabled/handling code in application code (though we may decide to leave it up to application code) how to render a disabled component.

Piccolo's solution was Node.toImage(), but this could also be implemented as 'new Image(Node)'.

Useful for reducing a complex collection of nodes to a single node, particularly at initialization time and/or when the nodes or collection are computed.

Currently needed in a couple of such places in beers-law-lab. See for example HorizontalTiledNode, which was ported from piccolo-phet.

Right now peers are just at top:0, left: 0, but some accessibility technologies may require the correct locations for the peers. We should investigate adding this support, with the caveat that it may harm performance, especially for accessible play area objects that move every frame, and especially on tablets.

In other event handling systems, down and up are matched pairs. Not so in scenery, which is more low level. If your node wants to receive an 'up' event, then your node's 'down' handler must register a new listener on the event.pointer received by the down handler. And you should take care to remove that listener on 'up' or 'cancel'.

Here's an example of how awkward/obfuscated this makes a down/up listener: (caveat: not tested code)

/*
* @param {function} downHandler function called on 'down', one {Event} arg
* @param {function} downHandler function called on 'up', one {Event} arg
*/ 
function DownUpListener( downHandler, upHandler ) {

    var downPointer;

    var cleanup = function() {
        if ( downPointer ) {
            downPointer.removeInputListener( upListener );
            downPointer = null;
        }
    };

    var upListener = function( event ) {
        this.up = function( event ) {
            upHandler( event ); // handle the 'up' event
            cleanup();
        }
        this.cancel = function() {
            cleanup();
        }   
    };

    this.down = function( event ) {
        downHandler( event ); // handle the 'down' event
        downPointer = event.downPointer; // remember pointer we're adding listener to
        downPoint.addListener( upListener ); // register up listener
    };

    this.cancel = function( event ) {
        cleanup();
    };
}

Try using SimpleDragHandler to drag a node that has constrained drag bounds. The cursor will become disconnected from the node.

Here's a use case (untested code). Note that node position is updated via a roundtrip to the model.

var mvt = new ModelViewTransform2D(…); // phetcommon
var location = new Property(…); // phetcommon
var node = new Text(…); // scenery

var constrainBounds = function ( point, bounds ) {
      if ( bounds === undefined || bounds.contains( point.x, point.y ) ) {
        return point;
      }
      else {
        var xConstrained = Math.max( Math.min( point.x, bounds.getMaxX() ), bounds.x );
        var yConstrained = Math.max( Math.min( point.y, bounds.getMaxY() ), bounds.y );
        return new Vector2( xConstrained, yConstrained );
      }
};

node.addInputListener( new SimpleDragHandler( {
     translate: function ( options ) {
          var pModel = mvt.viewToModel( new Vector2( options.position.x, options.position.y ) );
          var pModelConstrained = dragHandler.constrainBounds( pModel, movable.dragBounds );
          location.set( pModelConstrained );
        }
} );

location.addObserver( function( newLocation ) {
     var pView = mvt.modelToView( newLocation );
     node.x = pView.x;
     node.y = pView.y;
} );

Docs refer to 'slow' instead of 'accurate' as a text renderer setting. But elsewhere I think it reads 'accurate'. I'll change it.

When implementing input listeners (particularly drag handler), this type of code is currently required:

drag: function ( event, trail ) {
var localPosition = trail.getTransform().inversePosition2( event.pointer.point );
// ... then do something with the local position
}

event.pointer.point is in global coordinates, and (in this case) needs to be transformed to the node's local coordinate frame. Transforming to the parent's coordinate frame would be uglier.

Low priority since these coordinate-frame transforms are currently doable. But it would be nice to have something similar to Piccolo's globalToLocal, localToGlobal, localToParent, localToParent and parentToLocal.

From 4/16/13 email:

In BLL, I'm trying to instantiate a Text node like this:

var soluteLabel = new Text( "foo", { font: "32px Arial" });

I'm seeing an assertion failure with this message:

Bounds must be empty or finite in invalidateSelf

…and this stack trace:

(anonymous function) (assert.js:39)
Node.invalidateSelf (Node.js:363)
inherit.invalidateText (Text.js:76)
inherit.setFont (Text.js:292)
Node.mutate (Node.js:1443)
forEach (lodash-1.0.0-rc.3.js:2154)
Node.mutate (Node.js:1435)
Node (Node.js:120)
Text (Text.js:56)

Here's the assertion that's failing in Node.invalidateSelf:

invalidateSelf: function( newBounds ) {
assert && assert( newBounds.isEmpty() || newBounds.isFinite() , "Bounds must be empty or finite in invalidateSelf");

If I change the font to "22px Arial", the assertion doesn't fail, but the font size is clearly not rendered correctly. I have other instantiations of Text node that look just like this one, but don't exhibit the problem. All of my local repos are at latest. Canary 28.0.1480.0 and Safari 6.0.3 on Mac OS 10.8.3.

Any ideas?…

Scenery user input documentation (http://phetsims.github.io/scenery/doc/user-input.html) says this about the Event "target" property:

target The leaf-most Node in the Trail.

But examining an event in the debugger, it looks like target is currently a Trail (and possibly identical to the Event.trail property?)

Since validateBounds() is called again if a bounds listener changes its bounds, an infinite loop can occur (imagine listening to bounds changes on a node and increasing its scale no matter way).

We should detect these and throw an error instead of the infinite loop if possible, most likely only enabling the checking if assertions are enabled.

Most noticeable on Firefox Aurora 22.0a2 (build from 5 days ago) on Mac OS X, where on BLL the moving shaker text is much bolder than the stationary text.

Also confirmed on Firefox 20.0.1 on Windows 7 (64bit)

This problem is demonstrated in pixelzoom-tests/issues/scenery#13/.
It occurs on iPad using Safari mobile. I was testing on iPad2 with iOS 6.1.3.

A DOM node with a element is added to the scene as part of a more complex collection of nodes that will be dragged.

Two problems noted:
(1) The text lags behind the rest of the node during drags.
(2) The text on the is split across multiple lines.

Right now it is pretty difficult to convert bounds from one coordinate frame to another. It would be something like:

var global = getParent.localToGlobalBounds(this.bounds);
var transformed = other.globalToLocal(global);

I propose a pair of convenience functions to make this simpler:

//Get the Bounds2 of this node in the global coordinate frame.  Does not work for DAG.
get globalBounds(){
  assert && assert(this.parents.length===1,'globalBounds unable to work for DAG');
  return this.parents[0].localToGlobalBounds(this.bounds);
},

//Get the Bounds2 of any other node by converting to the global coordinate frame.  Does not work for DAG.
boundsOf: function(node){
  return this.globalToLocalBounds(node.globalBounds);
}

Using this latter convenience function, you can simply do:

//Gets the bounds of node b in a's coordinate frame.
a.boundsOf(b)

For future work, we could consider similar convenience functions for point or other transformations.

Currently we have:

scenery.Font.toString
scenery.Color.getCSS

Problems with this:

• toString is being hijacked for a specific purpose
• different names for the same functionality

I talked with JO about this, and we decided on toCSS.

Currently we're using a 'render-to-canvas-and-inspect' method that recursively determines text bounds down to the desired sub-pixel level. This is slow, but seems to be the only accurate method. Accuracy is needed for Canvas rendering of text, since we need to know the bounds for dirty region repainting (inaccurate bounds could lead to visual artifacts). There are a few options:

• Improve efficiency of the accurate bounds determination. We're scanning more pixels than necessary, and haven't looked at profiling information that could help tune the parameters involved. For example, we are also stretching the text in the non-measured direction to ensure that small pieces (like the dot on an 'i') do not get missed, but making this direction map to fewer pixels could cause a significant speedup.
• Use a combination of the faster DOM or SVG inspection methods to determine text bounds. These are inaccurate for a variety of edge cases including italics, combining characters, etc. Generally they always give the same height for the same font, regardless of the text content. To compensate, we would need to do one of the following:
  • Repaint entire canvases if they contain Text nodes, instead of repainting just the dirty regions. This could negatively impact performance, since the entire canvas would need to be cleared (even if it just contains a small text node).
  • Force non-canvas rendering for Text nodes (that would currently mean SVG). SVG text seems to be sharper on Chrome at least, so this may be a good option. It may help to have SVG be the default renderer so we don't slice up graphics-text-graphics into separate layers.

sun.PanelNode implements a control panel, by drawing a background around some "content node". It's desirable to be able to resize the control panel. Is it possible for PanelNode to somehow observe the (visible) bounds of the content node?

I could use this feature immediately in beers-law-lab, where the Wavelength control panel is supposed to resize depending on whether the wavelength slider is visible.

Piccolo's solution to this was via property listeners, and property PNode.PROPERTY_FULL_BOUNDS.

Available to anyone who wants to tackle it.

Scene.affectedLayers is called often to determine what layers are affected
by a change to a trail that affects children (transforms, visibility, etc.).
A layer is 'affected' any of its Trails that it paints are descendants of the
passed-in Trail.

It is currently taking ~13% of Scenery overhead in BLL's shaker animation,
since it isn't at all optimized yet.

Basically, the passed in Trail implicitly defines two TrailPointers (startPointer and
endPointer in the current code), and each Layer implicitly defines two
TrailPointers, currently in the loop as:
new scenery.TrailPointer( layer.startPaintedTrail, true ) and
new scenery.TrailPointer( layer.endPaintedTrail, false )

TrailPointer.compareNested( otherTrailPointer ) defines a total ordering over all
TrailPointers, and a layer is "affected" if startPointer <= layer's end pointer, and
endPointer >= layer's start pointer.

I'm imagining a binary search-style solution, where we find where those two constraints
switch from satisfied to unsatisfied, and then return all layers in-between that satisfy
both constraints.

Adding comprehensive unit test coverage to compare the faster implementation to the current
implementation would be good (probably using an example Scene layout like in test-utils.js'
createTestNodeTree(), but with a mixture of Paths and Nodes), and we might consider adding
in an assertion that verifies this at run-time (but is stripped out of the production version).

I'm also thinking about how a Layer could more efficiently track all of the Trails that
affect it, and add listeners to the relevant Nodes so that this method would be unnecessary,
however the trail-tracking in a Layer currently seems like it would add too much overhead
to the addition and removal of nodes in the graph.

node.setMatrix( Matrix3.IDENTITY ) is currently how this is done, but that's clunky.

There are two types of caching to be supported:

• Cache a node (and its subtree) at a specific resolution, draw to the canvas with the node's global transform (blurry if transform isn't an integral translation, may be pixelated on zoom, but doesn't need to update on a node or its ancestors being transformed) - may consider adaptive resolution for zooming? This would help BASE. Also consider snapping to integral translations?
• Cache an instance (and its subtree) in global coordinates, draw to the canvas with no transform. Always sharp as possible, but needs to be updated any time the node or one of its ancestors is transformed.

This issue is to track progress.

Currently, if the children of a node with <100% opacity are split across layers, those layers will individually blend incorrectly.

The solution would be to handle this case in the DOM. We would need to split layers before and after the Node, and create a div that would contain those interior layers, and give it a CSS opacity. This would allow the layers to blend at full alpha and then be processed correctly.

The div would need to have a z-index for the outside scope, but internally it should have its own zindices (see https://developer.mozilla.org/en-US/docs/CSS/Understanding_z-index/The_stacking_context)

Text node crashes on Mac OS 10.8 with Chrome 25.0.1364.152, Safari 6.0.2 and FireFox 19.0. The bounds returned by accurateCanvasBounds are NaN.

An isolated example is in repo pixelzoom-tests. See pixelzoom-tests/scenery-tests/Text-accurateCanvasBounds-bug/.

The workaround I'm using is to change accurateCanvasBounds to approximateSVBBounds in Text.prototype.invalidateText.

This is demonstrated in pixelzoom-tests/issues/scenery#14. Noted on iPad2 with iOS 6.1.3.

This example contains 1 Path node and 1 Text node. The Path has a SimpleDragHandler whose translate function updates the Text to indicate the coordinates of the drag position. So not a whole lot going on here. This example has nothing approaching smooth animation on iPad.

This is a simpler example of what I'm doing in ConcentrationMeterNode in beers-law-lab. In that example, dragging the probe updates the concentration value in the meter. No a lot of computation going on, and some notification via Property, but otherwise a similar example. In that case, the performance is so bad that the sim appears to be locked up, the probe can't be moved at all once it's place in the solution.

See ComboBoxNode in beers-law-lab. A combo box has 2 primary components: a button and a list. Pressing the button toggles whether the list is displayed. Mousing over the list highlights items, clicking an items selected it and hides the list. (The list is currently displayed/hidded using addChild/removeChild, since getBounds includes invisible children.)

To reproduce:

1. click the button to show the list
2. select an item in the list - this succeeds, the button changes and the list is hidden
3. click the button to show the list
4. notice that list items don't highlight and are not selectable
5. click the button to hide the list
6. click the button to show the list
7. select an item in the list - now it works

I'm not certain that this is a scenery problem.

Noted by CM:

Anyone else using Node.setVisible(false)? In some cases it's making the page totally unresponsive (no console output, Chrome eventually times out.) In other cases it's throwing an exception at CanvasLayer line 155, node.paintCanvas(state). Exception message is "Uncaught TypeError: Object [object Object] has no method 'paintCanvas'"

I'm seeing odd behavior when scaling a scenery Node with a negative value. I added a test case to the qunit tests that demonstrates the issue (commit forthcoming)...