search

phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

[Edge] Some tools and controls are hidden when there are "No Real Roots" #74

Closed phet-steele closed 5 years ago

phet-steele commented 5 years ago
  1. Standard Form screen.
  2. Enable Roots.
  3. Decrease "a" to 0.
  4. Increase "c" to 1.

image

I've found that "a" and "b" need to equal 0, and "c" must be non-zero for this to work. Other scenarios where "No Real Roots" occurs but these equation values are not met will not exhibit the bug (EDIT: https://github.com/phetsims/graphing-quadratics/issues/74#issuecomment-431425682). You will lose all tools/controls near the bottom of the screen, excluding the navbar. This includes: point tools, hide curve button, and Reset All button.

Now, these tools/controls can still be used! Move your mouse over their locations and you will see your cursor update to "the hand". I believe all we are seeing is an overly aggressive clip area. Evidence? Well, look at what happens if you drag the point tools onto the graph before performing the buggy procedure. Once the steps are followed, you can see that the tool gets clipped at the edges of the graph:

image

I see no errors while running the debug version. Seen on Win 10 Edge 42.17134.1.0. For phetsims/QA/issues/211.

Troubleshooting Information URL: https://phet-dev.colorado.edu/html/graphing-quadratics/1.0.0-dev.38/phet/graphing-quadratics_all_phet.html Version: 1.0.0-dev.38 2018-10-18 21:15:53 UTC Features missing: touch User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36 Edge/17.17134 Language: en-US Window: 1920x963 Pixel Ratio: 1/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 Vendor: Microsoft (Microsoft Edge) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"928741cf","branch":"master"},"axon":{"sha":"644353d6","branch":"master"},"brand":{"sha":"1fd6682e","branch":"master"},"chipper":{"sha":"fc5919bb","branch":"master"},"dot":{"sha":"dcccfddd","branch":"master"},"graphing-lines":{"sha":"08b2b43e","branch":"master"},"graphing-quadratics":{"sha":"9b807638","branch":"master"},"joist":{"sha":"b12072c7","branch":"master"},"kite":{"sha":"380cef53","branch":"master"},"phet-core":{"sha":"269c4c8a","branch":"master"},"phet-io":{"sha":"ce77e241","branch":"master"},"phet-io-wrapper-classroom-activity":{"sha":"534dc595","branch":"master"},"phet-io-wrapper-hookes-law-energy":{"sha":"3372700d","branch":"master"},"phet-io-wrapper-lab-book":{"sha":"09507f4d","branch":"master"},"phet-io-wrappers":{"sha":"0a4a3153","branch":"master"},"phetcommon":{"sha":"a2a1aaba","branch":"master"},"query-string-machine":{"sha":"9ae63aab","branch":"master"},"scenery":{"sha":"d7e6c53e","branch":"master"},"scenery-phet":{"sha":"344597e6","branch":"master"},"sherpa":{"sha":"0989d371","branch":"master"},"sun":{"sha":"289fce23","branch":"master"},"tambo":{"sha":"fe63321c","branch":"master"},"tandem":{"sha":"2c4b970a","branch":"master"}}
KatieWoe commented 5 years ago

Great catch @phet-steele. Can confirm issue on my device. Have not seen on any other browsers so far, including Internet Explorer. Will update issue if found elsewhere.

KatieWoe commented 5 years ago

Update: It seems that a and b do not need to be zero. It just needs to be positioned in a way that "No Real Roots" appears. setup

pixelzoom commented 5 years ago

Graphed curves (and their equations) are clipped to the graph using scenery's clipArea feature. Other elements on the graph (manipulators, vertex point, roots, "NO REAL ROOTS") are not clipped to the graph. And the point tools are Reset All button are certainly not clipped to anything.

I've looked through the sim code, and I don't see anything that I'm doing incorrectly wrt usage of clipArea. So it sounds like clipArea is misbehaving on Edge. @jonathanolson could you please investigate? clipArea is applied to the graph in GraphNode, line 69. The "NO REAL ROOTS" label is NoRealRootsNode, and it's added to StandardFormGraphNode (subclass of GraphNode for the "Standard Form" screen) on line 70.

pixelzoom commented 5 years ago

I just noticed this significant bit of info at the bottom of the report:

I see no errors while running the debug version.

So here we apparently have another case where the debug version behaves differently than the non-debug version.

phet-steele commented 5 years ago

So here we apparently have another case where the debug version behaves differently than the non-debug version.

No no no, what I mean is that I see no errors [in the console]. The bug still happens in debug.

phet-steele commented 5 years ago

I've found that "a" and "b" need to equal 0, and "c" must be non-zero for this to work.

Update: It seems that a and b do not need to be zero. It just needs to be positioned in a way that "No Real Roots" appears.

I could have sworn I saw a situation where "No Real Roots" was displayed and there was no issue, hence why I wrote that sentence. @pixelzoom @jonathanolson I think what I was seeing is if you enable the Vertex checkbox, the issue is remedied (except if "a" is zero and the curve has no vertex).

pixelzoom commented 5 years ago

Enabling the Vertex checkbox causes the vertex point to become visible on the graph, which probably triggers a redraw of the graph. None of the things that are disappearing (especially the Reset All button) are related to each other at all.

pixelzoom commented 5 years ago

This issue must be resolved before 1.0 publication.

jonathanolson commented 5 years ago

I'm aware of this, and will be investigating (potentially with a VM or a collaborator).

jonathanolson commented 5 years ago

Reproduced, and I'm also able to reproduce in Chrome with ?rootRenderer=canvas. Definitely seems to be a Scenery bug.

jonathanolson commented 5 years ago

Doing Scenery-internals work for this, I'm marking the issue as blocking publication until there has been a review done.

jonathanolson commented 5 years ago

Ok, it looks like my "safe patch" is the correct way to handle it long-term also.

Scenery uses a stack of Canvases for things like opacity (so that when it starts rendering stuff under a non-integer opacity Node, it pushes a new Canvas on the stack and starts drawing to it - when done it draws that Canvas into the next-most Canvas and pops it from the stack).

When pushing and popping Canvases to the stack, the "clipping area needs to be set for the current Canvas" flag was not being properly set in some cases. So to produce this bug, it needed a specific combination of opacity underneath a Node with a clipArea, such that the pop() would not make clipping dirty, so that things rendered after improperly had the clipping applied.

@pixelzoom can you verify that this issue is fixed?

@ariel-phet it would be nice to have someone to review the changes.

pixelzoom commented 5 years ago

I looked through the commits, but reviewing the changes is beyond my understanding of scenery.

I don't have an Edge test platform, so (after doing pull-all.sh) I've built 1.0.0-dev.42. @phet-steele could you please verify?

KatieWoe commented 5 years ago

@pixelzoom it looks fixed to me.

pixelzoom commented 5 years ago

Great, thanks. Closing.