FreeForm Improvements.

[veillette]

There has been many performance improvement to this sim in #243 and #210 ( Thanks @pixelzoom !). However, an unfortunate side effect of this performance is the responsiveness of the free drag: because the free drag mode is so responsive, it creates a curve that is less smooth than before, which makes the first and second derivative more noisy.

Currently, the freeform mode takes the three previous dragPoints and change the y-values of the curvePoints that are encompassed by these three drag points. We carry different kind of operations on the y-value of these curvePoints to make the curve smooth. However, the improve responsiveness means that the "window" of curvePoints is much narrower, and therefore, our smoothing is not as effective.

[pixelzoom]

Brainstorming... Instead of creating a point each time the DragListener's drag function is called, should we set a maximum for the number of points that we sample during some time interval?

[veillette]

should we set a maximum for the number of points that we sample during some time interval?

Oh hmm, I don't know what that means. We should talk. One idea that I had is to keep track of the first point of the start drag. This point gets "lost" since we keep only the three most recent dragPosition.

[veillette]

@pixelzoom and I discussed his idea in https://github.com/phetsims/calculus-grapher/issues/297#issuecomment-1474018230. I say that I would try to do a version of this which would be based on space interval rather than time interval.

[veillette]

The following curve is obtained from https://phet-dev.colorado.edu/html/calculus-grapher/1.0.0-dev.25/phet/calculus-grapher_en_phet.html while drawing with freeform very slowly. Notice the noise in the derivative.

[amanda-phet]

I just wanted to chime in to say that a student in an interview described this as "the computer picking up every little pixel" or something like that. He recognized what was happening, and while he didn't like it he also wasn't confused by it. He was very appreciative of the smooth tool (although we had to point it out to him).

I don't mean to stop you from improving this, just wanted to share that interview note.

[veillette]

Thanks for the note @amanda-phet..

[veillette]

The commits above prevent an update of the curve if two points are to close to one another, while doing a drag in freeform. It leads to a bit of an apparent unresponsiveness in free form as the cursor does not quite follow the curve, but can lag by about $$ \Delta X=0.1$$ . However, we can reduce the noise by a considerable factor with this relatively approach.

[veillette]

      // Do not update the curve model if the drag points in (freeform mode) are too close from one another,
      // to prevent noise in the derivative  (see https://github.com/phetsims/calculus-grapher/issues/297 )
      if ( penultimatePosition === null || curveManipulationProperties.mode !== CurveManipulationMode.FREEFORM ||
           Math.abs( modelPosition.x - penultimatePosition.x ) > 0.1 ) {

        // Update curve based on mode and width
        interactiveCurveNodeProperty.value.transformedCurve.manipulateCurve(
          curveManipulationProperties.mode,
          curveManipulationProperties.width,
          modelPosition,
          penultimatePosition,
          antepenultimatePosition );

        // Update (model) antepenultimatePosition and penultimatePosition
        antepenultimatePosition = penultimatePosition;
        penultimatePosition = modelPosition;
      }

@pixelzoom : Although I am relatively happy with the end result, I think this could be cleaned up a bit better, code-wise. Ideally , we would put the if-statement within the manipulate method of TransformedCurve but I could not figure out what to do with the update for antepenultimatePosition and penultimatePosition without replicating the same if statement. We should discuss.

[pixelzoom]

We should discuss.

I should have some time first thing Thursday 3/23 morning (Eastern time). Look for me on Slack, and let's talk.

[pixelzoom]

The code that @veillette referenced in https://github.com/phetsims/calculus-grapher/issues/297#issuecomment-1480181753 is in OriginalGraphNode.ts. The conditional would be nice to move into code that is specific to free-form, like drawFreeformToPosition. But we need to update antepenultimatePosition and penultimatePosition.

I'll have a look.

[pixelzoom]

In https://github.com/phetsims/calculus-grapher/commit/b23b14310682e8c73d9e0ca0ea9e8de11e1a478c, @veillette and I paired on improving the readability of OriginalGraphNode updateCurve.

Back to @veillette for another look. Feel free to close.

[veillette]

That looks good. Closing.

[KatieWoe]

Noting that I did see a difference in https://github.com/phetsims/qa/issues/924 compared to the last dev test. I don't think it needs to be addressed but noting here.