Assertion failed: Not handling arcs with start/end angles that show differences in-between browser handling

[pixelzoom]

Related to phetsims/qa#445 and phetsims/qa#446.

Discovered during investigation of #260.

This occurs on macOS 10.14.6 with Chrome 77 & 78, Safari 13.0.1, and Firefox 70 (and I suspect other supported platforms.)

It occurs in master bb1e85e and 1.0.0-rc.1/vector-addition_all_phet_debug.html?ea

Steps to reproduce:

1. start sim with ?ea
2. go to Equations screen
3. Select the radio button for "Polar" coordinates (pink icon at lower right)
4. expand the "Base Vector" accordion box
5. set picker for 'd' magnitude to 1
6. set picker for 'e' magnitude to 1
7. set picker for 'e' theta to 85
8. set picker for 'd' theta to -95. The assertion failure shown below will occur when transitioning from -90 to -95.

This happens when 'd' and 'e' magnitude pickers are equal and 1, 2, or 3. It does not happen when they are 4-10, 0, or any negative value.

Chrome stack trace:

assert.js?bust=1572307060629:22 Uncaught Error: Assertion failed: Not handling arcs with start/end angles that show differences in-between browser handling
    at window.assertions.assertFunction (assert.js?bust=1572307060629:22)
    at Arc.invalidate (Arc.js?bust=1572307060705:333)
    at new Arc (Arc.js?bust=1572307060705:53)
    at Subpath.offset (Subpath.js?bust=1572307060705:481)
    at Shape.getOffsetShape (Shape.js?bust=1572307060705:1486)
    at SumVectorNode.updateVector (RootVectorNode.js?bust=1572307060705:147)
    at RootVectorNode.js?bust=1572307060705:108
    at listener (Multilink.js?bust=1572307060705:41)
    at TinyEmitter.emit (TinyEmitter.js?bust=1572307060705:68)
    at DerivedProperty._notifyListeners (Property.js?bust=1572307060705:265)

[pixelzoom]

The related code is RootVectorNode line 145:

      // Make the arrow easier to grab by setting pointer areas
      if ( rootVector.magnitude > 0 && this.arrowNode.shape ) {
        this.arrowNode.mouseArea = this.arrowNode.shape.getOffsetShape( VectorAdditionConstants.VECTOR_MOUSE_AREA_DILATION );
        this.arrowNode.touchArea = this.arrowNode.shape.getOffsetShape( VectorAdditionConstants.VECTOR_TOUCH_AREA_DILATION );
      }

Inspecting rootVector.magnitude, its value is 9.71445146547012e-17.

[pixelzoom]

KITE/Arc invalidate method has these assertions, and the first one is failing here. I don't understand the assertion expression or message. I'm going to ask @jonathanolson for assistance.

      // Constraints that should always be satisfied
      assert && assert( !( ( !this.anticlockwise && this._endAngle - this._startAngle <= -Math.PI * 2 ) ||
                           ( this.anticlockwise && this._startAngle - this._endAngle <= -Math.PI * 2 ) ),
        'Not handling arcs with start/end angles that show differences in-between browser handling' );
      assert && assert( !( ( !this.anticlockwise && this._endAngle - this._startAngle > Math.PI * 2 ) ||
                           ( this.anticlockwise && this._startAngle - this._endAngle > Math.PI * 2 ) ),
        'Not handling arcs with start/end angles that show differences in-between browser handling' );

[jonathanolson]

Noted in slack:

Ahh, basically if you have a full circle of an arc, there is an edge case where one browser shows it one way, and another browser shows it a different way. What code is triggering this? It can probably be resolved by slightly tweaking how the arc is constructed. And then it should be documented better how to do that

This looks like something I should handle, since it seems to be happening in a getOffsetShape call.

[pixelzoom]

The relevant code is noted above, RootVectorNode, where arrowNode is an instance of SCENERY_PHET/ArrowNode.

And thanks for handling!

[pixelzoom]

This is the last issue holding up the next RC, and we need to move on. So I've pushed a workaround in the commits above to master and 1.0 branches. This workaround is OK as a permanent solution, since other parts of the code use VectorAdditionConstants.ZERO_THRESHOLD to treat magnitude as "effectively zero".

@jonathanolson if you want to investigate further, rollback the above changes in a branch. FYI, the assertion failure was occurring in the code shown in #261 (comment) when rootVector.magnitude was very small (9.71445146547012e-17) and the dimensions of arrowNode were consequently very small (1.5157520574185608e-15 x 12).

[pixelzoom]

The general issue has been moved to phetsims/kite#82.

It's not necessary to have @arouinfar or QA review, because this very obviously fails or doesn't fail with one specific set of conditions, which I've tested. So moving this issue to "fixed-awaiting-deploy", and it will be regressed in the next RC cycle.

[KatieWoe]

This no longer seems to cause an assertion failure in rc.2