Add support for values decreasing to the left/up

samreid commented 3 years ago

From https://github.com/phetsims/bamboo/issues/27#issuecomment-861887346 @pixelzoom said:

https://github.com/phetsims/bamboo/issues/38 got me to thinking about an AxesNode with doubleHead: false, and wondering if it did the right thing -- the arrow head should point in the axis' positive direction. AxisNode implementation looks like it always assumes +x right, +y up. And then I realized that ChartTransform is limited to +x right, +y up because it uses Range for modelXRange and modelYRange. This seems like a serious limitiation of a chart library. Should it be addressed? @samreid thoughts?

I can imagine that we would need a chart which has decreasing values going up vertically. (Not sure about decreasing values going to the left), but I think we should add support for both.

samreid commented 3 years ago

I made good progress creating an Interval type, but after discussion with @pixelzoom we are wondering if this can be accomplished in the transform.

Here's a patch that uses Interval:

```diff Index: main/fourier-making-waves/js/wavepacket/view/ComponentsChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/wavepacket/view/ComponentsChartNode.js b/main/fourier-making-waves/js/wavepacket/view/ComponentsChartNode.js --- a/main/fourier-making-waves/js/wavepacket/view/ComponentsChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/wavepacket/view/ComponentsChartNode.js (date 1623862271253) @@ -47,8 +47,8 @@ // ChartTransform options transformOptions: { - modelXRange: new Range( 0, 1 ), - modelYRange: new Range( 0, 1 ), + modelXInterval: new Range( 0, 1 ), + modelYInterval: new Range( 0, 1 ), viewWidth: 100, viewHeight: 100 }, @@ -109,7 +109,7 @@ const value = ( domain === Domain.TIME ) ? T : L; const xMin = value * xAxisDescription.range.min; const xMax = value * xAxisDescription.range.max; - chartTransform.setModelXRange( new Range( xMin, xMax ) ); + chartTransform.setModelXInterval( new Range( xMin, xMax ) ); xGridLines.setSpacing( xAxisDescription.gridLineSpacing * value ); xTickMarks.setSpacing( xAxisDescription.tickMarkSpacing * value ); xTickLabels.setSpacing( xAxisDescription.tickLabelSpacing * value ); Index: main/bamboo/js/Interval.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/Interval.js b/main/bamboo/js/Interval.js new file mode 100644 --- /dev/null (date 1623862987526) +++ b/main/bamboo/js/Interval.js (date 1623862987526) @@ -0,0 +1,41 @@ +// Copyright 2021, University of Colorado Boulder + +/** + * Defines a start and end value, used for choosing the left/right or top/bottom values in a plot. + * + * @author Sam Reid (PhET Interactive Simulations) + */ + +import bamboo from './bamboo.js'; + +class Interval { + + /** + * @param {number} startValue + * @param {number} endValue + */ + constructor( startValue, endValue ) { + this.startValue = startValue; + this.endValue = endValue; + } + + get min() { + return Math.min( this.startValue, this.endValue ); + } + + get max() { + return Math.max( this.startValue, this.endValue ); + } + + /** + * @param {Interval} interval + * @returns {boolean} + * @public + */ + equals( interval ) { + return interval.startValue === this.startValue && interval.endValue === this.endValue; + } +} + +bamboo.register( 'Interval', Interval ); +export default Interval; \ No newline at end of file Index: main/fourier-making-waves/js/common/view/AmplitudesChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/common/view/AmplitudesChartNode.js b/main/fourier-making-waves/js/common/view/AmplitudesChartNode.js --- a/main/fourier-making-waves/js/common/view/AmplitudesChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/common/view/AmplitudesChartNode.js (date 1623862271248) @@ -65,8 +65,8 @@ tandem: Tandem.REQUIRED }, options ); - assert && assert( !options.transformOptions.modelXRange, 'AmplitudesChartNode sets modelXRange' ); - assert && assert( !options.transformOptions.modelYRange, 'AmplitudesChartNode sets modelYRange' ); + assert && assert( !options.transformOptions.modelXInterval, 'AmplitudesChartNode sets modelXInterval' ); + assert && assert( !options.transformOptions.modelYInterval, 'AmplitudesChartNode sets modelYInterval' ); // Fields of interest in amplitudesChart, to improve readability const fourierSeries = amplitudesChart.fourierSeries; @@ -74,8 +74,8 @@ // the transform from model to view coordinates const chartTransform = new ChartTransform( merge( { - modelXRange: new Range( 1 - X_MARGIN, fourierSeries.harmonics.length + X_MARGIN ), - modelYRange: fourierSeries.amplitudeRange + modelXInterval: new Range( 1 - X_MARGIN, fourierSeries.harmonics.length + X_MARGIN ), + modelYInterval: fourierSeries.amplitudeRange }, options.transformOptions ) ); const chartRectangle = new ChartRectangle( chartTransform ); Index: main/circuit-construction-kit-common/js/view/CCKCChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/circuit-construction-kit-common/js/view/CCKCChartNode.js b/main/circuit-construction-kit-common/js/view/CCKCChartNode.js --- a/main/circuit-construction-kit-common/js/view/CCKCChartNode.js (revision 65cfa35d4aa4c5d6a0b7595204567f3cf589e0dc) +++ b/main/circuit-construction-kit-common/js/view/CCKCChartNode.js (date 1623862271246) @@ -126,8 +126,8 @@ const chartTransform = new ChartTransform( { viewWidth: 150, viewHeight: 100, - modelXRange: new Range( 0, 4.25 ), - modelYRange: new Range( -2, 2 ) + modelXInterval: new Range( 0, 4.25 ), + modelYInterval: new Range( -2, 2 ) } ); const chartBackground = new ChartRectangle( chartTransform, { fill: 'white', @@ -202,7 +202,7 @@ } } ); zoomLevelProperty.link( zoomLevel => { - chartTransform.setModelYRange( zoomRanges[ zoomLevel ] ); + chartTransform.setModelYInterval( zoomRanges[ zoomLevel ] ); verticalGridLineSet.setSpacing( zoomRanges[ zoomLevel ].max / 2 ); verticalLabelSet.setSpacing( zoomRanges[ zoomLevel ].max / 2 ); } ); @@ -229,7 +229,7 @@ timeProperty.link( time => { // Show 4 seconds, plus a lead time of 0.25 sec - chartTransform.setModelXRange( new Range( time - 4, time + 0.25 ) ); + chartTransform.setModelXInterval( new Range( time - 4, time + 0.25 ) ); verticalGridLineSet.setLineDashOffset( time * chartTransform.modelToViewDelta( Orientation.HORIZONTAL, 1 ) ); updatePen(); } ); Index: main/fourier-making-waves/js/wavepacket/view/WavePacketAmplitudesChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/wavepacket/view/WavePacketAmplitudesChartNode.js b/main/fourier-making-waves/js/wavepacket/view/WavePacketAmplitudesChartNode.js --- a/main/fourier-making-waves/js/wavepacket/view/WavePacketAmplitudesChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/wavepacket/view/WavePacketAmplitudesChartNode.js (date 1623862271245) @@ -44,8 +44,8 @@ // ChartTransform options transformOptions: { - modelXRange: new Range( 0, 1 ), - modelYRange: new Range( 0, 1 ), + modelXInterval: new Range( 0, 1 ), + modelYInterval: new Range( 0, 1 ), viewWidth: 100, viewHeight: 100 }, @@ -112,7 +112,7 @@ //TODO replace this with auto scaling //TODO this is a problem - AxisDescription assumes range is symmetric, which is not the case in Wave Packet screen - chartTransform.setModelYRange( new Range( 0, yAxisDescription.range.max ) ); + chartTransform.setModelYInterval( new Range( 0, yAxisDescription.range.max ) ); // Grid lines and tick marks are determined by AxisDescriptions regardless of whether auto scale is enabled. // This is because the model keeps AxisDescriptions in sync with yAxisAutoScaleRange. Index: main/fourier-making-waves/js/common/view/SumChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/common/view/SumChartNode.js b/main/fourier-making-waves/js/common/view/SumChartNode.js --- a/main/fourier-making-waves/js/common/view/SumChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/common/view/SumChartNode.js (date 1623861771433) @@ -76,10 +76,10 @@ [ yAutoScaleProperty, yAxisAutoScaleRangeProperty ], ( yAutoScale, yAxisAutoScaleRange ) => { if ( yAutoScale ) { - this.chartTransform.setModelYRange( yAxisAutoScaleRange ); + this.chartTransform.setModelYInterval( yAxisAutoScaleRange ); } else { - // Do not setModelYRange when auto scale becomes false. We want the range to remain unchanged + // Do not setModelYInterval when auto scale becomes false. We want the range to remain unchanged // until the user explicitly changes it via the y-axis zoom buttons. } } ); Index: main/bamboo/js/ChartTransform.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/ChartTransform.js b/main/bamboo/js/ChartTransform.js --- a/main/bamboo/js/ChartTransform.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/ChartTransform.js (date 1623863742960) @@ -8,7 +8,6 @@ */ import Emitter from '../../axon/js/Emitter.js'; -import Range from '../../dot/js/Range.js'; import Transform1 from '../../dot/js/Transform1.js'; import Util from '../../dot/js/Utils.js'; import Vector2 from '../../dot/js/Vector2.js'; @@ -16,6 +15,7 @@ import Orientation from '../../phet-core/js/Orientation.js'; import bamboo from './bamboo.js'; import ClippingType from './ClippingType.js'; +import Interval from './Interval.js'; class ChartTransform { @@ -28,12 +28,12 @@ // The horizontal axis is referred to as the "x" axis, though it may be used to depict another dimension, such as "time" viewWidth: 100, // {number} width in view coordinates - modelXRange: new Range( -1, 1 ), // {Range} range of the x axis, in model coordinates + modelXInterval: new Interval( -1, 1 ), // {Interval} interval of the x axis, in model coordinates xTransform: new Transform1( x => x, x => x ), // {Transform1} model-to-view scaling function for the x axis // The vertical axis is referred to as the "y" axis, though it may be used to depict another dimension such as "width" viewHeight: 100, // {number} height in view coordinates - modelYRange: new Range( -1, 1 ), // {Range} range of the y axis, in model coordinates + modelYInterval: new Interval( -1, 1 ), // {Interval} interval of the y axis, in model coordinates yTransform: new Transform1( x => x, x => x ) // {Transform1} model-to-view scaling function for the y axis }, options ); @@ -46,8 +46,8 @@ // @public (read-only) this.viewWidth = options.viewWidth; this.viewHeight = options.viewHeight; - this.modelXRange = options.modelXRange; - this.modelYRange = options.modelYRange; + this.modelXInterval = options.modelXInterval; + this.modelYInterval = options.modelYInterval; // @private this.xTransform = options.xTransform; @@ -62,7 +62,7 @@ } /** - * For the axis that corresponds to Orientation, iterates over the range and performs an operation (specified by + * For the axis that corresponds to Orientation, iterates over the interval and performs an operation (specified by * callback) at regular intervals (specified by spacing). * @param {Orientation} axisOrientation * @param {number} spacing - the spacing (delta) between operations, in model coordinates @@ -73,9 +73,9 @@ */ forEachSpacing( axisOrientation, spacing, origin, clippingType, callback ) { - const modelRange = this.getModelRange( axisOrientation ); - const nMin = getValueForSpacing( modelRange.min, clippingType, origin, spacing, Math.ceil ); - const nMax = getValueForSpacing( modelRange.max, clippingType, origin, spacing, Math.floor ); + const modelInterval = this.getModelInterval( axisOrientation ); + const nMin = getValueForSpacing( modelInterval.min, clippingType, origin, spacing, Math.ceil ); + const nMax = getValueForSpacing( modelInterval.max, clippingType, origin, spacing, Math.floor ); for ( let n = nMin; n <= nMax + 1E-6; n++ ) { const modelPosition = n * spacing + origin; @@ -135,7 +135,7 @@ modelToView( axisOrientation, value ) { assert && assert( Orientation.includes( axisOrientation ), `invalid axisOrientation: ${axisOrientation}` ); - const modelRange = axisOrientation === Orientation.HORIZONTAL ? this.modelXRange : this.modelYRange; + const modelInterval = axisOrientation === Orientation.HORIZONTAL ? this.modelXInterval : this.modelYInterval; const viewDimension = axisOrientation === Orientation.HORIZONTAL ? this.viewWidth : this.viewHeight; const transform = axisOrientation === Orientation.HORIZONTAL ? this.xTransform : this.yTransform; @@ -146,8 +146,8 @@ // For vertical, +y is usually up return axisOrientation === Orientation.HORIZONTAL ? - Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), 0, viewDimension, transformedValue ) : - Util.linear( transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), 0, viewDimension, transformedValue ); + Util.linear( transform.evaluate( modelInterval.startValue ), transform.evaluate( modelInterval.endValue ), 0, viewDimension, transformedValue ) : + Util.linear( transform.evaluate( modelInterval.endValue ), transform.evaluate( modelInterval.startValue ), 0, viewDimension, transformedValue ); } /** @@ -160,14 +160,14 @@ viewToModel( axisOrientation, value ) { assert && assert( Orientation.includes( axisOrientation ), `invalid axisOrientation: ${axisOrientation}` ); - const modelRange = axisOrientation === Orientation.HORIZONTAL ? this.modelXRange : this.modelYRange; + const modelInterval = axisOrientation === Orientation.HORIZONTAL ? this.modelXInterval : this.modelYInterval; const viewDimension = axisOrientation === Orientation.HORIZONTAL ? this.viewWidth : this.viewHeight; const transform = axisOrientation === Orientation.HORIZONTAL ? this.xTransform : this.yTransform; // For vertical, +y is usually up const out = axisOrientation === Orientation.HORIZONTAL ? - Util.linear( 0, viewDimension, transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), value ) : - Util.linear( 0, viewDimension, transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), value ); + Util.linear( 0, viewDimension, transform.evaluate( modelInterval.startValue ), transform.evaluate( modelInterval.endValue ), value ) : + Util.linear( 0, viewDimension, transform.evaluate( modelInterval.endValue ), transform.evaluate( modelInterval.startValue ), value ); return transform.inverse( out ); } @@ -228,38 +228,38 @@ } /** - * Sets the Range for the model's x dimension. - * @param {Range} modelXRange + * Sets the Interval for the model's x dimension. + * @param {Interval} modelXInterval * @public */ - setModelXRange( modelXRange ) { - if ( !modelXRange.equals( this.modelXRange ) ) { - this.modelXRange = modelXRange; + setModelXInterval( modelXInterval ) { + if ( !modelXInterval.equals( this.modelXInterval ) ) { + this.modelXInterval = modelXInterval; this.changedEmitter.emit(); } } /** - * Sets the Range for the model's y dimension. - * @param {Range} modelYRange + * Sets the Interval for the model's y dimension. + * @param {Interval} modelYInterval * @public */ - setModelYRange( modelYRange ) { - if ( !modelYRange.equals( this.modelYRange ) ) { - this.modelYRange = modelYRange; + setModelYInterval( modelYInterval ) { + if ( !modelYInterval.equals( this.modelYInterval ) ) { + this.modelYInterval = modelYInterval; this.changedEmitter.emit(); } } /** - * Gets the model range for the axis that corresponds to Orientation. + * Gets the model Interval for the axis that corresponds to Orientation. * @param {Orientation} axisOrientation - * @returns {Object.modelXRange|Object.modelYRange} + * @returns {Interval} * @public */ - getModelRange( axisOrientation ) { + getModelInterval( axisOrientation ) { assert && assert( Orientation.includes( axisOrientation ), `invalid axisOrientation: ${axisOrientation}` ); - return ( axisOrientation === Orientation.HORIZONTAL ) ? this.modelXRange : this.modelYRange; + return ( axisOrientation === Orientation.HORIZONTAL ) ? this.modelXInterval : this.modelYInterval; } /** Index: main/bamboo/js/demo/DemoUpDownArrowPlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoUpDownArrowPlot.js b/main/bamboo/js/demo/DemoUpDownArrowPlot.js --- a/main/bamboo/js/demo/DemoUpDownArrowPlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoUpDownArrowPlot.js (date 1623862271250) @@ -21,20 +21,20 @@ constructor( options ) { super(); - const modelXRange = new Range( 0, 10 ); - const modelYRange = new Range( -100, 100 ); + const modelXInterval = new Range( 0, 10 ); + const modelYInterval = new Range( -100, 100 ); // one data point for each integer point in the model, y values interpolated along the x range from min to max const dataSet = []; - for ( let i = 0; i <= modelXRange.max; i++ ) { - dataSet.push( new Vector2( i, modelYRange.min + ( modelYRange.getLength() / modelXRange.getLength() ) * i ) ); + for ( let i = 0; i <= modelXInterval.max; i++ ) { + dataSet.push( new Vector2( i, modelYInterval.min + ( modelYInterval.getLength() / modelXInterval.getLength() ) * i ) ); } const chartTransform = new ChartTransform( { viewWidth: 500, viewHeight: 400, - modelXRange: modelXRange, - modelYRange: modelYRange + modelXInterval: modelXInterval, + modelYInterval: modelYInterval } ); const chartRectangle = new ChartRectangle( chartTransform, { @@ -51,8 +51,8 @@ }, pointToPaintableFields: point => { - // interpolate from red at modelYRange.min, green at modelYRange.max - const distance = 1 / ( modelYRange.getLength() ) * ( point.y - modelYRange.min ); + // interpolate from red at modelYInterval.min, green at modelYInterval.max + const distance = 1 / ( modelYInterval.getLength() ) * ( point.y - modelYInterval.min ); const c = Color.interpolateRGBA( new Color( 'red' ), new Color( 'lawngreen' ), distance ); return { fill: c }; } Index: main/fourier-making-waves/js/common/view/WaveformChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/common/view/WaveformChartNode.js b/main/fourier-making-waves/js/common/view/WaveformChartNode.js --- a/main/fourier-making-waves/js/common/view/WaveformChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/common/view/WaveformChartNode.js (date 1623862271244) @@ -66,13 +66,13 @@ tandem: Tandem.REQUIRED }, options ); - assert && assert( !options.transformOptions.modelXRange, 'WaveformChartNode sets modelXRange' ); - assert && assert( !options.transformOptions.modelYRange, 'WaveformChartNode sets modelYRange' ); + assert && assert( !options.transformOptions.modelXInterval, 'WaveformChartNode sets modelXInterval' ); + assert && assert( !options.transformOptions.modelYInterval, 'WaveformChartNode sets modelYInterval' ); // the transform between model and view coordinate frames const chartTransform = new ChartTransform( merge( { - modelXRange: xAxisDescriptionProperty.value.createAxisRange( domainProperty.value, L, T ), - modelYRange: yAxisDescriptionProperty.value.range + modelXInterval: xAxisDescriptionProperty.value.createAxisRange( domainProperty.value, L, T ), + modelYInterval: yAxisDescriptionProperty.value.range }, options.transformOptions ) ); // The chart's background rectangle @@ -141,7 +141,7 @@ const value = ( domain === Domain.TIME ) ? T : L; const xMin = value * xAxisDescription.range.min; const xMax = value * xAxisDescription.range.max; - chartTransform.setModelXRange( new Range( xMin, xMax ) ); + chartTransform.setModelXInterval( new Range( xMin, xMax ) ); xGridLines.setSpacing( xAxisDescription.gridLineSpacing * value ); xTickMarks.setSpacing( xAxisDescription.tickMarkSpacing * value ); xTickLabels.setSpacing( xAxisDescription.tickLabelSpacing * value ); @@ -188,7 +188,7 @@ // Range is determined by yAxisDescription only if auto scale is disabled. if ( !yAutoScaleProperty || !yAutoScaleProperty.value ) { - chartTransform.setModelYRange( yAxisDescription.range ); + chartTransform.setModelYInterval( yAxisDescription.range ); } // Grid lines and tick marks are determined by AxisDescriptions regardless of whether auto scale is enabled. Index: main/bamboo/js/LinearEquationPlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/LinearEquationPlot.js b/main/bamboo/js/LinearEquationPlot.js --- a/main/bamboo/js/LinearEquationPlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/LinearEquationPlot.js (date 1623862271248) @@ -95,8 +95,8 @@ // Slope is infinite, draw a vertical line. const modelX = 0; - const modelMinY = this.chartTransform.modelYRange.min; - const modelMaxY = this.chartTransform.modelYRange.max; + const modelMinY = this.chartTransform.modelYInterval.min; + const modelMaxY = this.chartTransform.modelYInterval.max; // Convert to view coordinates. const viewX = this.chartTransform.modelToViewX( modelX ); @@ -108,9 +108,9 @@ else { // Determine the endpoints, in model coordinates. - const modelMinX = this.chartTransform.modelXRange.min; + const modelMinX = this.chartTransform.modelXInterval.min; const modelMinY = this.solve( modelMinX ); - const modelMaxX = this.chartTransform.modelXRange.max; + const modelMaxX = this.chartTransform.modelXInterval.max; const modelMaxY = this.solve( modelMaxX ); // Convert to view coordinates. Index: main/bamboo/js/demo/DemoScatterPlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoScatterPlot.js b/main/bamboo/js/demo/DemoScatterPlot.js --- a/main/bamboo/js/demo/DemoScatterPlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoScatterPlot.js (date 1623862271250) @@ -38,8 +38,8 @@ const chartTransform = new ChartTransform( { viewWidth: 600, viewHeight: 400, - modelXRange: new Range( -1, 1 ), - modelYRange: new Range( -1, 1 ) + modelXInterval: new Range( -1, 1 ), + modelYInterval: new Range( -1, 1 ) } ); const chartRectangle = new ChartRectangle( chartTransform, { @@ -101,7 +101,7 @@ // Controls const centerXProperty = new NumberProperty( 0 ); - centerXProperty.link( centerX => chartTransform.setModelXRange( new Range( -1 - centerX, 1 - centerX ) ) ); + centerXProperty.link( centerX => chartTransform.setModelXInterval( new Range( -1 - centerX, 1 - centerX ) ) ); const controls = new HSlider( centerXProperty, new Range( -1.25, 1.25 ), { trackSize: new Dimension2( 500, 5 ) } ); Index: main/fourier-making-waves/js/wavepacket/view/WavePacketScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/wavepacket/view/WavePacketScreenView.js b/main/fourier-making-waves/js/wavepacket/view/WavePacketScreenView.js --- a/main/fourier-making-waves/js/wavepacket/view/WavePacketScreenView.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/wavepacket/view/WavePacketScreenView.js (date 1623862271247) @@ -65,8 +65,8 @@ // Amplitudes chart const amplitudesChartNode = new WavePacketAmplitudesChartNode( model.amplitudesChart, { transformOptions: { - modelXRange: new Range( 0, model.significantWidth ), - modelYRange: new Range( 0, model.maxAmplitude ), //TODO this needs to autoscale! + modelXInterval: new Range( 0, model.significantWidth ), + modelYInterval: new Range( 0, model.maxAmplitude ), //TODO this needs to autoscale! viewWidth: DiscreteScreenView.CHART_RECTANGLE_SIZE.width, viewHeight: DiscreteScreenView.CHART_RECTANGLE_SIZE.height }, @@ -95,8 +95,8 @@ // Components chart const componentsChartNode = new ComponentsChartNode( model.componentsChart, { transformOptions: { - modelXRange: new Range( -2, 2 ), //TODO - modelYRange: new Range( -model.maxAmplitude, model.maxAmplitude ), //TODO + modelXInterval: new Range( -2, 2 ), //TODO + modelYInterval: new Range( -model.maxAmplitude, model.maxAmplitude ), //TODO viewWidth: DiscreteScreenView.CHART_RECTANGLE_SIZE.width, viewHeight: DiscreteScreenView.CHART_RECTANGLE_SIZE.height }, @@ -127,8 +127,8 @@ // Sum chart const sumChartNode = new WavePacketSumChartNode( model.sumChart, { transformOptions: { - modelXRange: new Range( -2, 2 ), //TODO - modelYRange: new Range( -1.25, 1.25 ), //TODO + modelXInterval: new Range( -2, 2 ), //TODO + modelYInterval: new Range( -1.25, 1.25 ), //TODO viewWidth: DiscreteScreenView.CHART_RECTANGLE_SIZE.width, viewHeight: DiscreteScreenView.CHART_RECTANGLE_SIZE.height }, Index: main/bamboo/js/demo/DemoBarPlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoBarPlot.js b/main/bamboo/js/demo/DemoBarPlot.js --- a/main/bamboo/js/demo/DemoBarPlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoBarPlot.js (date 1623862271252) @@ -46,8 +46,8 @@ const chartTransform = new ChartTransform( { viewWidth: 700, viewHeight: 300, - modelXRange: new Range( 0, Math.PI * 24 ), - modelYRange: new Range( 0, 0.14 ) + modelXInterval: new Range( 0, Math.PI * 24 ), + modelYInterval: new Range( 0, 0.14 ) } ); const chartRectangle = new ChartRectangle( chartTransform, { Index: main/fourier-making-waves/js/wavepacket/view/WavePacketSumChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/fourier-making-waves/js/wavepacket/view/WavePacketSumChartNode.js b/main/fourier-making-waves/js/wavepacket/view/WavePacketSumChartNode.js --- a/main/fourier-making-waves/js/wavepacket/view/WavePacketSumChartNode.js (revision 8fe8d74ef3645cee58ad0364a0dde4fd3b251435) +++ b/main/fourier-making-waves/js/wavepacket/view/WavePacketSumChartNode.js (date 1623862271246) @@ -47,8 +47,8 @@ // ChartTransform options transformOptions: { - modelXRange: new Range( 0, 1 ), - modelYRange: new Range( 0, 1 ), + modelXInterval: new Range( 0, 1 ), + modelYInterval: new Range( 0, 1 ), viewWidth: 100, viewHeight: 100 }, @@ -109,7 +109,7 @@ const value = ( domain === Domain.TIME ) ? T : L; const xMin = value * xAxisDescription.range.min; const xMax = value * xAxisDescription.range.max; - chartTransform.setModelXRange( new Range( xMin, xMax ) ); + chartTransform.setModelXInterval( new Range( xMin, xMax ) ); xGridLines.setSpacing( xAxisDescription.gridLineSpacing * value ); xTickMarks.setSpacing( xAxisDescription.tickMarkSpacing * value ); xTickLabels.setSpacing( xAxisDescription.tickLabelSpacing * value ); Index: main/bamboo/js/demo/DemoLinearEquationPlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoLinearEquationPlot.js b/main/bamboo/js/demo/DemoLinearEquationPlot.js --- a/main/bamboo/js/demo/DemoLinearEquationPlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoLinearEquationPlot.js (date 1623862271249) @@ -38,22 +38,22 @@ options = merge( {}, options ); - const modelXRange = new Range( -50, 50 ); - const modelYRange = new Range( -50, 50 ); + const modelXInterval = new Range( -50, 50 ); + const modelYInterval = new Range( -50, 50 ); const tickXSpacing = 10; const tickYSpacing = 10; // Keep the same aspect ratio for view range const viewScale = 3; - const viewWidth = viewScale * modelXRange.getLength(); - const viewHeight = viewScale * modelYRange.getLength(); + const viewWidth = viewScale * modelXInterval.getLength(); + const viewHeight = viewScale * modelYInterval.getLength(); // Properties for slope (m) and y-intercept (b). const mProperty = new NumberProperty( 1, { range: new Range( -10, 10 ) } ); const bProperty = new NumberProperty( 0, { - range: modelYRange + range: modelYInterval } ); // Add some markup, so they these like math symbols. @@ -112,8 +112,8 @@ } ); const chartTransform = new ChartTransform( { - modelXRange: modelXRange, - modelYRange: modelYRange, + modelXInterval: modelXInterval, + modelYInterval: modelYInterval, viewWidth: viewWidth, viewHeight: viewHeight } ); Index: main/greenhouse-effect/js/common/view/EnergyBalancePanel.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/greenhouse-effect/js/common/view/EnergyBalancePanel.js b/main/greenhouse-effect/js/common/view/EnergyBalancePanel.js --- a/main/greenhouse-effect/js/common/view/EnergyBalancePanel.js (revision 8238d2b72cec6da57e26f740262466c2f39d3239) +++ b/main/greenhouse-effect/js/common/view/EnergyBalancePanel.js (date 1623862271251) @@ -111,9 +111,9 @@ const chartTransform = new ChartTransform( { viewWidth: PLOT_VIEW_WIDTH, - modelXRange: horizontalModelRange, + modelXInterval: horizontalModelRange, viewHeight: PLOT_VIEW_HEIGHT, - modelYRange: new Range( 0, verticalModelRange ) + modelYInterval: new Range( 0, verticalModelRange ) } ); // the dataSet for the barPlot gets set in a multilink of the provided energy Properties Index: main/bamboo/js/demo/DemoChartCanvasNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoChartCanvasNode.js b/main/bamboo/js/demo/DemoChartCanvasNode.js --- a/main/bamboo/js/demo/DemoChartCanvasNode.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoChartCanvasNode.js (date 1623862271251) @@ -47,8 +47,8 @@ const chartTransform = new ChartTransform( { viewWidth: 700, viewHeight: 300, - modelXRange: new Range( -Math.PI / 8, Math.PI / 8 ), - modelYRange: new Range( -4 / Math.PI, 4 / Math.PI ) + modelXInterval: new Range( -Math.PI / 8, Math.PI / 8 ), + modelYInterval: new Range( -4 / Math.PI, 4 / Math.PI ) } ); const chartRectangle = new ChartRectangle( chartTransform, { @@ -66,7 +66,7 @@ bottom: chartRectangle.bottom } ); zoomLevelProperty.link( zoomLevel => { - chartTransform.setModelXRange( zoomLevel === 1 ? new Range( -Math.PI / 8, Math.PI / 8 ) : + chartTransform.setModelXInterval( zoomLevel === 1 ? new Range( -Math.PI / 8, Math.PI / 8 ) : zoomLevel === 2 ? new Range( -Math.PI / 4, Math.PI / 4 ) : zoomLevel === 3 ? new Range( -Math.PI / 3, Math.PI / 3 ) : zoomLevel === 4 ? new Range( -Math.PI / 2, Math.PI / 2 ) : null ); Index: main/bamboo/js/demo/DemoLinePlot.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoLinePlot.js b/main/bamboo/js/demo/DemoLinePlot.js --- a/main/bamboo/js/demo/DemoLinePlot.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoLinePlot.js (date 1623862271249) @@ -43,8 +43,8 @@ const chartTransform = new ChartTransform( { viewWidth: 700, viewHeight: 300, - modelXRange: new Range( -Math.PI / 8, Math.PI / 8 ), - modelYRange: new Range( -4 / Math.PI, 4 / Math.PI ) + modelXInterval: new Range( -Math.PI / 8, Math.PI / 8 ), + modelYInterval: new Range( -4 / Math.PI, 4 / Math.PI ) } ); const chartRectangle = new ChartRectangle( chartTransform, { @@ -62,7 +62,7 @@ bottom: chartRectangle.bottom } ); zoomLevelProperty.link( zoomLevel => { - chartTransform.setModelXRange( + chartTransform.setModelXInterval( zoomLevel === 1 ? new Range( -Math.PI / 8, Math.PI / 8 ) : zoomLevel === 2 ? new Range( -Math.PI / 4, Math.PI / 4 ) : zoomLevel === 3 ? new Range( -Math.PI / 3, Math.PI / 3 ) : Index: main/bamboo/js/demo/DemoMultiplePlots.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/bamboo/js/demo/DemoMultiplePlots.js b/main/bamboo/js/demo/DemoMultiplePlots.js --- a/main/bamboo/js/demo/DemoMultiplePlots.js (revision 477bf2a87a23d69175da7ca7b88b642da1534690) +++ b/main/bamboo/js/demo/DemoMultiplePlots.js (date 1623863692359) @@ -26,6 +26,7 @@ import ChartRectangle from '../ChartRectangle.js'; import ClippingType from '../ClippingType.js'; import GridLineSet from '../GridLineSet.js'; +import Interval from '../Interval.js'; import LabelSet from '../LabelSet.js'; import LinePlot from '../LinePlot.js'; import ScatterPlot from '../ScatterPlot.js'; @@ -44,8 +45,10 @@ const chartTransform = new ChartTransform( { viewWidth: 600, viewHeight: 400, - modelXRange: new Range( 2, 10 ), - modelYRange: new Range( 1, 22000 ), + modelXInterval: new Interval( 10, 2 ), + modelYInterval: new Interval( 1, 22000 ), + + // TODO: option doesn't exist yScale: new Transform1( Math.log, Math.exp, { domain: new Range( 0, Number.POSITIVE_INFINITY ), range: new Range( 0, Number.POSITIVE_INFINITY ) ```

Instead, we are wondering if we can accomplish this by keeping the same Range in the model, but specifying the transform function so that it can accomplish this flip. I'll take a look.

@pixelzoom also indicated that this isn't critical to implement immediately, but will be good to do before we have a sim that needs this feature.

samreid commented 3 years ago

I think I have a better understanding of what is happening here. There are 2 transforms at play. The first, such as xTransform is a model-based transform that specifies a mapping for the data. The second is a linear transform which is currently hardcoded like so:

    // For vertical, +y is usually up
    return axisOrientation === Orientation.HORIZONTAL ?
           Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), 0, viewDimension, transformedValue ) :
           Util.linear( transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), 0, viewDimension, transformedValue );

Note that (in the x-direction) the linear functions map the minimum (transformed) model value to the left edge and the maximum (transformed) model value to the right edge. We can flip the axis if we invert this linear transform like so:

// Map the minimum of the transformed model range to the right edge
Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), viewDimension, 0, transformedValue )

To summarize, there are 2 transforms:

A possibly nonlinear model-ish transform that determines the mapping of the data
A hardcoded linear transform that determines where the values are shown in the view. Both of these are used in mapping model points to view points.

Would it be possible to somehow combine these? How would that even look? If we keep them separate, what should the API be for allowing the client to customize the latter transform?

pixelzoom commented 3 years ago

Having 2 places where the transform is handled is definitely problematic. What we seem to need is something like ModelViewTransform2 that handles non-linear transforms. bamboo will be significantly disadvantaged if we're constrained to linear mappings with +x left and +y up. Let's add @jonathanolson to the discussion to see if he has any ideas.

jonathanolson commented 3 years ago

What do you need out of the transforms? Do we NEED to know when it's linear, for optimizations? Could you just accept any invertible function (presumably specifying a function and its inverse?)

samreid commented 3 years ago

Please review https://github.com/phetsims/bamboo/issues/39#issuecomment-862602848 which describes the nonlinear model-oriented transform (which is supported by function + inverse), and the subsequent linear view-oriented transform (which "sizes" the transformed data to fit the chart view). It makes sense to separate these 2 transforms, so that the nonlinear model transform can be specified independently of the size of the chart.

jonathanolson commented 2 years ago

So if we have separate transforms, the potentially non-invertible one could presumably just have a similar API to LinearFunction?

e.g. accept

export type InvertibleFunction = {
  evaluate( n: number ): number;
  inverse( n: number): number;
};

so then LinearFunction would satisfy it automatically?

I've also used map/inverse before.

ModelViewTransform2's API would be fairly difficult to generalize to arbitrary nonlinear mappings. It would require a few derivatives to be computed, for shape/bounds it would probably require discretization (or custom logic), etc.

samreid commented 2 years ago

I don't think we want a fully general nonlinear API for the view transform. (That is already supported in the model transform). Instead, we would like to say that instead of the chart going from (-4,4) we want it to go from (4,-4). However, if we also want to power it with our Range class that won't be possible since range requires min<max. Instead, I added booleans:

  public modelXRangeInverted: boolean;
  public modelYRangeInverted: boolean;

And tested that both are working well in the demos. I left an example of the x one committed, and it looks like this:

For the y one I tested temporarily and confirmed it was working nicely, but decided not to commit since I feel the upside-up one is a better initial demo.

@pixelzoom would you like to review?

pixelzoom commented 2 years ago

I guess that's sufficient. We'll see how it works out when someone needs an inverted range in practice. Closing.

phetsims / bamboo

Add support for values decreasing to the left/up #39