Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Still Possible to make sharp vertex #440

Open
KatieWoe opened this issue Oct 3, 2018 · 12 comments
Open

Still Possible to make sharp vertex #440

KatieWoe opened this issue Oct 3, 2018 · 12 comments

Comments

@KatieWoe
Copy link
Contributor

KatieWoe commented Oct 3, 2018

Test device:
Dell Laptop
Operating System:
Win 10
Browser:
Chrome
Problem description:
For phetsims/qa#134 after discussion in #418 (comment).
Related to #193.
It is possible to still create sharp vertex in the playground screen. The solution to the previous issue seemed to be that if there was a sharp point in the track a movable point would jump to a new position, eliminating the point. However, if that point jumps to a position that also creates a sharp point, it is not recognized and remains sharp.
Steps to reproduce:

  1. Go to the playground screen and connect two pieces of track at least.
  2. Create a zigzag pattern with the track. IE. the first point is near the bottom of the screen, the second is near the top, the third is near the bottom, etc.
  3. Move the points back and forth so that several sharp points are created. If the subsequent jump does not create a point, move the points back in to position. Unfortunately, this seems to be largely by chance. The more sharp points you make originally the greater the chance that it will happen.
  4. This may take several tries, as the video bellow shows.

Screenshots:
sharp vertex
Video: https://drive.google.com/file/d/1R64SEbwm_CE0nETaQLbzdD9plVwN7kl_/view?usp=sharing

Troubleshooting information (do not edit):

Name: ‪Energy Skate Park: Basics‬
URL: https://phet-dev.colorado.edu/html/energy-skate-park-basics/1.4.0-dev.1/phet/energy-skate-park-basics_en_phet.html
Version: 1.4.0-dev.1 2018-06-20 00:09:06 UTC
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36
Language: en-US
Window: 1536x732
Pixel Ratio: 2.5/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: 4096
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":"f0522e7c","branch":"master"},"brand":{"sha":"89d28f63","branch":"master"},"chipper":{"sha":"e8946524","branch":"master"},"dot":{"sha":"6482f8c9","branch":"master"},"energy-skate-park-basics":{"sha":"288fcefd","branch":"master"},"joist":{"sha":"22e437d5","branch":"master"},"kite":{"sha":"a1086efc","branch":"master"},"phet-core":{"sha":"17326041","branch":"master"},"phet-io":{"sha":"3ea0727a","branch":"master"},"phet-io-wrapper-classroom-activity":{"sha":"53708616","branch":"master"},"phet-io-wrapper-hookes-law-energy":{"sha":"8a546a32","branch":"master"},"phet-io-wrapper-lab-book":{"sha":"1527afd6","branch":"master"},"phet-io-wrappers":{"sha":"8d814eab","branch":"master"},"phetcommon":{"sha":"6ec8cd89","branch":"master"},"query-string-machine":{"sha":"4182612f","branch":"master"},"scenery":{"sha":"88cb642e","branch":"master"},"scenery-phet":{"sha":"7bcde0b2","branch":"master"},"sherpa":{"sha":"88c3b828","branch":"master"},"sun":{"sha":"7579e8fa","branch":"master"},"tandem":{"sha":"8461b6f3","branch":"master"}}

@jessegreenberg
Copy link
Contributor

Regarding sharp track kinks, @ariel-phet said in #434

The super sharp kink is not very pedagogically useful. I agree it would be nice if it were disallowed, but it definitely was tricky to get the current algorithm for track smoothing working as @samreid pointed out. You are definitely welcome to work more on this problem, but I think I would give it a limit. Something like 5-6 hours of investigation, and if you don't have a promising solution/direction, leave it alone.

I definitely do not think you need to worry about some odd behavior while dragging the track. If a kink or such happens while the skater is on the track and the track is being dragged, I don't think we will easily be able to avoid all those potential edge cases. If you work on (1) only worry about static cases. If some weirdness happens in some edge cases while the skater is moving and the track is being dragged, that is acceptable.

@jessegreenberg
Copy link
Contributor

In the tracks found in #434, the smoothing algorithm is returning that smoothing was successful, so I would like to understand why this is considered smooth:
capture

@jessegreenberg
Copy link
Contributor

The correction loop:

 while ( this.getMinimumRadiusOfCurvature() < MAXIMUM_ACCEPTABLE_RADIUS_OF_CURVATURE && numTries < MAX_TRIES ) {...}

this.getMinimumRadiusOfCurvature() is 0.031, MAXIUMUM_ACCEPTABLE_RADIUS_OF_CURVATURE is 0.3.

@jessegreenberg
Copy link
Contributor

The problem may be with getMinRadiusOfCurvature, which can return larger numbers occasionally like in #440 (comment), and then much smaller numbers with seemingly identical tracks, causing a re-smooth.

@jessegreenberg
Copy link
Contributor

If I increase numDivisions in getMinimumRadiusOfCurvature to 800, curvatures are more accurate. But smoothPointOfHighestCurvature is now unable to find a control point.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 16, 2018

I am abut 2 hours into this, which helped me understand how the current algoritm works and try a couple of solutions.

From this issue and #434, there are two reasons that kinks might show up, the track is incorrectly considered smooth by the model, and the model is unable to smooth a kinked track.

The first case will happen because of the resolution of getMinimumRadiusOfCurvature, which breaks the spline into divisions and finds the smallest radius by calculating radius at each division. For cases like #440 (comment), the spike is in between two divisions and it is missed. I tried doubling/tripling the number of divisions, and this fixed one test track I added, but I can still consistently create tracks like the one in #440 (comment). And increasing this value significantly will probably have performance implications. I wonder if it is possible to do more searching around local max/min of the track?

For the second case, I tried adding a second correction step where we find the point of highest curvature then if surrounded by two other points, try adjusting the surrounding points to "pull apart" the kink. This worked OK in some cases, but wasn't able to correct tracks like the one in #440 (comment). And sometimes pulling apart more than one control point made radically different tracks than what I was trying to create.

Ill spend a little more time trying to find radii along the spline focusing on local minima/maxima for the first case, but I won't spend more time on the second case for now.

@jessegreenberg
Copy link
Contributor

I wonder if it is possible to do more searching around local max/min of the track?

I tried this and it is working well so I am going to commit. In getMinimumRadiusOfCurvature and getUWithHighestCurvature, we find local minima and maxima and scour those areas with smaller divisions so that we are less likely to miss small radius of curvature when there are spikes in the track. I am no longer able to produce tracks like #440 (comment).

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 16, 2018

I don't think this will have much of a performance impact because these functions currently use 400 samples, and this only adds 10 samples per local extrema.

I am still able to produce cases like #440 (comment), the case where smooth is unable to smooth the track. I am out of time from #440 (comment) and don't have any good ideas to handle that case. If we want to address it, it may be helpful to discuss with @samreid or others at a design meeting how to prevent non-pedagogically useful tracks.

@jessegreenberg
Copy link
Contributor

Pinging @ariel-phet and @samreid so they are aware of this status update, and adding the deferred label, we may pick this up again in ESP.

@samreid
Copy link
Member

samreid commented Oct 18, 2018

Got it, thanks @jessegreenberg. I'll wait to hear from @ariel-phet if I should help on this issue.

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Feb 5, 2020

Occurs on phetsims/qa#470

@marlitas
Copy link
Contributor

It seems like this issue is probably still outside the scope of the upcoming region and culture publication. Leaving as deferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants