phetsims / molecules-and-light

"Molecules and Light" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 5 forks source link

Uncaught Error: Assertion failed: matrix should be a finite Matrix3 #373

Closed jessegreenberg closed 4 years ago

jessegreenberg commented 4 years ago

Caught from https://github.com/phetsims/aqua/issues/106, trying to enable multitouch fuzzing.

molecules-and-light
Uncaught Error: Assertion failed: matrix should be a finite Matrix3
Error: Assertion failed: matrix should be a finite Matrix3
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at Node.setMatrix (http://10.0.0.179:8080/scenery/js/nodes/Node.js:2420:15)
    at Node.set matrix [as matrix] (http://10.0.0.179:8080/scenery/js/nodes/Node.js:2425:30)
    at AnimatedPanZoomListener.reposition (http://10.0.0.179:8080/scenery/js/listeners/MultiListener.js:472:29)
    at AnimatedPanZoomListener.reposition (http://10.0.0.179:8080/scenery/js/listeners/PanZoomListener.js:105:11)
    at AnimatedPanZoomListener.addPress (http://10.0.0.179:8080/scenery/js/listeners/MultiListener.js:365:10)
    at AnimatedPanZoomListener.addPress (http://10.0.0.179:8080/scenery/js/listeners/PanZoomListener.js:91:11)
    at AnimatedPanZoomListener.addPress (http://10.0.0.179:8080/scenery/js/listeners/AnimatedPanZoomListener.js:633:11)
    at AnimatedPanZoomListener.convertBackgroundPresses (http://10.0.0.179:8080/scenery/js/listeners/MultiListener.js:458:12)
    at Object.move (http://10.0.0.179:8080/scenery/js/listeners/MultiListener.js:180:18)
jessegreenberg commented 4 years ago

The matrix it is trying to set is full of NaNs.

jessegreenberg commented 4 years ago

Seemed to happen quickly in the above comment, but now after many minutes of fuzzing I cannot get this to happen.

jessegreenberg commented 4 years ago

I suspect it is a division by zero error

    let localSquaredDistance = 0;
    let targetSquaredDistance = 0;

    localPoints.forEach( localPoint => { localSquaredDistance += localPoint.distanceSquared( localCentroid ); } );
    targetPoints.forEach( targetPoint => { targetSquaredDistance += targetPoint.distanceSquared( targetCentroid ); } );

    let scale = Math.sqrt( targetSquaredDistance / localSquaredDistance );

local points could be right on the localCentroid and distance could possibly end up as zero.

But then the entries would be Infinity rather than NaN I think. So perhaps that isn't what is happening.

jessegreenberg commented 4 years ago

OK, I just hit this assertion: assert && assert( localSquaredDistance !== 0, 'distance to centroid should not be zero' );

happened while I was resizing window if that matters.

jessegreenberg commented 4 years ago

Detailed infor:

There are two Presses down. Each have same trail with id 215-216, going to simulation root. First one at (81,205). Second one at (166,584). Local points are BOTH at {x: 50.30255742309739, y: 413.06044413233985}. Target points are same as Pointer points.

I don't think this is the problem, the scale is limited. Stepping beyond the assertion shows no problems.

jessegreenberg commented 4 years ago

OK, hit the actual error - this time two Pointers are down in the exact same spot: image

jessegreenberg commented 4 years ago

Should be fixed in the above commit - if Press points are in exactly the same spot (which is only possible during fuzz testing) we will not adjust scale at all. I tested by trying ?fuzz&fuzzPointers=3 for many minutes, and in aqua (where it seemed to happen more frequently. Can no longer get it to occur. Closing.