SoundParticleLayer: issues with createSphereImage

[pixelzoom]

Related to code review #259, noted while investigating how I might use CanvasNode to render particles in Gas Properties.

A couple of issues with function createSphereImagein SoundParticleLayer, which looks like this:

  /**
   * Create an image of a ShadedSphereNode for the given color.
   * @param {ColorDef} color
   * @returns {HTMLImageElement|HTMLCanvasElement}
   */
  const createSphereImage = color => new ShadedSphereNode( 10, {
    mainColor: color,
    stroke: 'black'
  } ).rasterized( {
    resolution: RESOLUTION,
    useCanvas: true
  } ).children[ 0 ].image;

(1) Node's rasterized method is documented as "Returns a node (backed by a scenery Image) that is a rasterized version of this node." and @returns {Node}. That does not translate to rasterized().children[ 0 ].image. That is not part of the public interface, you had to look at the implementation to pull that out, and I don't believe it's something you should be doing. Wave Interference appears to be the only sim using this approach, as there are zero other occurrences of ).children[ 0 ] in PhET code. Node toCanvas is the method that I’ve used with CanvasNode in other sims, and that appears to be what's used in other sims - I see 14 occurrences of .toCanvas(. Highly recommended to consult with @jonathanolson on this.

(2) The return type should be {HTMLCanvasElement}, not {HTMLImageElement|HTMLCanvasElement}. Verified in console. This is a minor issue, but I thought it might be worth noting.

[samreid]

@jonathanolson what do you recommend for this case?

[jonathanolson]

It looks like it's extracting the Canvas out, so directly using node.toCanvas(...) looks preferred in general.

[samreid]

Proposed fix is committed. I also temporarily turned randomness to 0 to verify the grid locations were not disturbed, and it looked good. @pixelzoom can you please review?

[pixelzoom]

👍 Closing.