phetsims / blackbody-spectrum

"Blackbody Spectrum" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 3 forks source link

Both saved curves can appear as a solid line when zoomed in #43

Closed phet-steele closed 5 years ago

phet-steele commented 5 years ago

At max Y zoom and 5 levels out from that, both saved curves will appear as solid lines. In other words:

  1. Zoom the Y axis all the way in.
  2. Save a curve.
  3. Adjust temperature.
  4. Save your second curve.

Once you have zoomed out far enough, one will revert to correctly be dashed.

image

Seen on Win 10 Chrome. For phetsims/QA/issues/215.

Troubleshooting Information URL: https://phet-dev.colorado.edu/html/blackbody-spectrum/1.0.0-dev.12/phet/blackbody-spectrum_en_phet.html Version: 1.0.0-dev.12 2018-11-02 23:46:56 UTC Features missing: touch User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.67 Safari/537.36 Language: en-US Window: 1920x938 Pixel Ratio: 1/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4095 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"928741cf","branch":"master"},"axon":{"sha":"cca8e36e","branch":"master"},"blackbody-spectrum":{"sha":"3db65ad6","branch":"master"},"brand":{"sha":"1fd6682e","branch":"master"},"chipper":{"sha":"fa2fbadf","branch":"master"},"dot":{"sha":"bbbd8526","branch":"master"},"joist":{"sha":"82521d0c","branch":"master"},"kite":{"sha":"380cef53","branch":"master"},"phet-core":{"sha":"1b90ac2f","branch":"master"},"phet-io":{"sha":"e230c094","branch":"master"},"phet-io-wrapper-classroom-activity":{"sha":"246085c1","branch":"master"},"phet-io-wrapper-hookes-law-energy":{"sha":"7479b0ec","branch":"master"},"phet-io-wrapper-lab-book":{"sha":"c46f7839","branch":"master"},"phet-io-wrappers":{"sha":"ded21e3f","branch":"master"},"phetcommon":{"sha":"cd63d89a","branch":"master"},"query-string-machine":{"sha":"06ed6276","branch":"master"},"scenery":{"sha":"5013941c","branch":"master"},"scenery-phet":{"sha":"2286294f","branch":"master"},"sherpa":{"sha":"97e70540","branch":"master"},"sun":{"sha":"a423abfb","branch":"master"},"tambo":{"sha":"296cf3ba","branch":"master"},"tandem":{"sha":"1b93c58b","branch":"master"}}
arnabp commented 5 years ago

@jbphet suggested this might be a scenery issue. @jonathanolson any idea what's going on here?

jonathanolson commented 5 years ago

This is particularly interesting, since the behavior seems to be the same cross-browser, AND the actual SVG element still seems to have the correct style information (unchanged with zoom): style="fill: none;stroke: gray;stroke-width: 5;stroke-linejoin: round;stroke-miterlimit: 10;stroke-dasharray: 5,5;stroke-dashoffset: 0;"

It's likely that there is a cut-off in the SVG spec. Maybe the curve is maxing out the "path length", or the dashes are very small compared to the length. Investigating.

jonathanolson commented 5 years ago

It appears that the cutoff is almost assuredly "if the dash pattern would be repeated 1 million or more times to cover the entire curve, don't show dashes".

We use a 5,5 dash pattern (5-length stroked, 5-length gap). Here is the information for the first three zoom settings that cause problems:

14128530.975705406 length, 7,7 NO, 8,8 OK (change from ~1009180 to ~883033 repeats) 70642596.58993901 length 35,35 NO, 36,36 OK (change from ~1009179 to ~981147 repeats) 353212929.39047116 length 176,176 NO, 177,177 OK (change from ~1003445 to 997776 repeats)

jonathanolson commented 5 years ago

Also a side-note that performance seems to be significantly worse when the browser has to compute how to fit slightly less than 1,000,000 repeats of a dash pattern, particularly for the longer paths. It took seconds (!) for Chrome to update the dash when testing the last option above.

jonathanolson commented 5 years ago

Running the changes through testing, but I'll be adding shape.shapeClip( clipShape ). For example the following code (in the kite playground) which takes a grid and clips it with a shape composed of two circles:

window.clipShape = new kite.Shape();
clipShape.circle( 0, 0, 10 ).moveTo( 40, 0 ).circle( 30, 0, 10 );
window.shape = new kite.Shape();
for ( var i = -30; i <= 30; i += 1 ) {
shape.moveTo( -20, i ).lineTo( 100, i );
}
window.result = shape.shapeClip( clipShape, {
  includeExterior: false,
  includeInterior: true,
  includeBoundary: true
} );

var node = new scenery.Node( {
  scale: 5,
  x: 80,
  y: 80
} );
node.addChild( new scenery.Path( result, { stroke: 'red', lineWidth: 0.1 } ) );
node.addChild( new scenery.Path( clipShape, { stroke: 'blue', lineWidth: 0.1 } ) );
scene.addChild( node );
display.updateDisplay();

results in:

screen shot 2018-11-07 at 2 55 41 pm

The provided shapeClip options are the defaults, but you can also clip by excluding the interior and including the exterior instead (the inverse shape).

I'll push once it passes tests.

jonathanolson commented 5 years ago

Pushed (passed snapshot comparisons, fuzzing, etc.)

jonathanolson commented 5 years ago

Leaving assigned to me to add unit tests. @arnabp let me know if using this works out ok.

arnabp commented 5 years ago

Clipping looks good, dashed lines stay dashed at all wavelengths now

arnabp commented 5 years ago

@jonathanolson Unfortunately looks like clipping breaks the intensity shape fill. Instead of closing the single subpath, it now tries to close the two subpaths separately and fill them, creating this mess.

screen shot 2018-11-14 at 1 34 19 pm

Since this is a subpath issue it doesn't look like I can handle this on blackbody side without using private functions, unless I make a new shape alongside the original that adds a lineTo from when it moves outside of the clip shape to when it moves back in (which feels like it defeats the purpose of using shapeClip if I'm just doing it manually). Any ideas?

jonathanolson commented 5 years ago

The above commit should probably fix the remaining issues related to internal errors with kite for this.

I'd use shapeClip only for the stroke. If you need a filled area also, use shapeIntersection (or just use the non-clipped version of it, since it was probably working before).

The shape intersection would leave a stroke on the bottom, since it turns non-closed shapes into closed shapes, so it won't work for the main stroke.

arnabp commented 5 years ago

After using shapeIntersection we ran into some issues with edge cases being handles by scenery. Some of these bugs were handled, but the last one went accordingly:

Add this.intensityPath.shape = this.intensityPath.shape.shapeIntersection( clipShape ); to line 206 of blackbody-spectrum/view/GraphDrawingNode.js. Zoom out once on the y axis and twice on the x axis, which produces a "TODO" error in Vertex.js in Kite.

jbphet commented 5 years ago

@arnabp and I discussed this with @jonathanolson and he recommended that we drop the idea of trying to use shapeIntersection and do the clipping explicitly in the blackbody sim code.

arnabp commented 5 years ago

This has been implemented using clipping within blackbody.