Feature/296-pinch-and-zoom #297

brett-estabrook · 2021-11-01T00:15:56Z

I would say PR #291 is a prequisite for this.

This is my initial pull request for pinch and zoom. I believe it is feature complete at this point.

When testing I recommend setting the following to true to enable Pinch and Zoom on all charts:

Microcharts/Sources/Microcharts/Charts/AxisBasedChart.cs

Line 136 in f6fd13a

    
           public bool EnableZoom { get; set; } = false; //FIXME: This should be off by default, but easier to test charts with it on

Charts tested:

Platform tested :

Android
iOS
MacOS
Windows

Fix:

Adding test data

Cache maxvalue, minvalue, and valuerange to prevent constant recalculations during drawing loops

Removing excess logging and adding better data generation

Better test data. Fixing issue with user defined min/max values Adding an event for chart rendering complete

Proper set comparison Don't double change animation progress when updating series

The lines and bar charts were drawing too close to left labels

Adding some comments

Making the values match the labels

Fixing issue with null values

Adding some comments about min/max values

…ue-perf-fixes

…-max-value-perf-fixes" This reverts commit 695537c, reversing changes made to 9a570c7.

Making labels match values

Initial support for ClipRect and Pinch to Zoom

Addressing an issue with line charts a indexing into draw points

…iprect

Handle scaling drawing of axis labels Improved clip rects

Applying correct equality check Moving label drawing into its own loop Fix label count in chart dynamic data

…iprect

Calculate height of Y Axis Labels and pad cliprect

EnableZoom property, XAxisMaxLabels allows to not draw all labels when over dense data points Zoom on Y Axis ticks and X Axis labels

Adding support for panning

brett-estabrook · 2021-11-01T00:23:43Z

Sources/Microcharts/Charts/AxisBasedChart.cs

+        /// </summary>
+        public bool EnableZoom { get; set; } = false; //FIXME: This should be off by default, but easier to test charts with it on
+
+        public ChartXForm XForm { get; } = new ChartXForm();


Not sure if this should be public or not, I could see renaming EnableZoom to EnableXForm and allowing users to do what they want...

what benefit would this give the user? I'm thinking it could be private

Currently ChartView modifies it based on the Pinch Gesture. We'd need to resolve that, thinking chart could maybe have an init function and then the chart sets the gesture on the ChartView.

As for giving user's access, I could see a benefit to manually changing the Chart Transform. Some kind of animation or custom user input.

brett-estabrook · 2021-11-01T00:24:18Z

Sources/Microcharts/Charts/AxisBasedChart.cs

+        /// <summary>
+        /// How many labels to draw, -1 means all of them
+        /// </summary>
+        public int XAxisMaxLabels { get; set; } = -1;


I added this for densely packed points. So we can reduce the draw on the horizontal axis. -1 will draw all labels by default.

brett-estabrook · 2021-11-01T00:25:11Z

Sources/Microcharts/Charts/AxisBasedChart.cs

+                if (EnableZoom && !fixedRange)
+                {
+                    //WARNING: This will change Min/Max based on a scale of 1.0
+                    MeasureHelper.CalculateYAxis(ShowYAxisText, ShowYAxisLines, yAxisFormat, YAxisMaxTicks, YAxisTextPaint, YAxisPosition, width, fixedRange, ref maxValue, ref minValue, out yAxisXShift, out yAxisIntervalLabels);


I calculate a NiceMin/NiceMax once for a scale of 1.0 then use that as a base when recalculating the yAxis while zoomed

brett-estabrook · 2021-11-01T00:25:55Z

Sources/Microcharts/Charts/AxisBasedChart.cs

+
+                SKRect chartRect = new SKRect(canvasRect.Left+yAxisXShift+Margin, canvasRect.Top+headerWithLegendHeight, width, chartMinY);
+                SKRect labelRect = new SKRect(chartRect.Left, canvasRect.Top + chartMinY, chartRect.Right, canvasRect.Bottom);
+                SKRect yAxisRect = new SKRect(canvasRect.Left, chartRect.Top - (yAxisSize.Height/2.0f), canvasRect.Right, chartRect.Bottom + (yAxisSize.Height/2.0f));


I only pad the yAxis clip rect, and only if we're drawing labels. The height is 0 when we're not drawing the yAxis.

brett-estabrook · 2021-11-01T00:32:00Z

Sources/Microcharts.Forms/ChartView.cs

+                if( axisChart != null && axisChart.EnableZoom )
+                {
+                    //FIXME: how to handle disable zoom after already enabled
+                    var pinchGesture = new PinchGestureRecognizer();


I wonder if this should be moved into the AxisBasedChart and then have the ChartView call an init function on the chart.

brett-estabrook · 2021-11-01T00:33:38Z

Sources/Microcharts/Charts/AxisBasedChart.cs

+
+                float yAxisXShift;
+                List<float> yAxisIntervalLabels;
+                string yAxisFormat = XForm.Scale == 1.0f ? "G" : "F1"; //As we scale the yAxis we get more decimal places


When zooming in I will Scale the Y Axis to draw more lines. However, this sometimes results in Y Axis values with lots of decimal points (1.667 etc) so I set a format string. I could see an end user wanting to configure this.

andrveld · 2022-02-25T19:50:01Z

Are there plans for this to be merged soon?

eman1986 · 2022-02-25T19:57:01Z

I was trying to let another person on the team review this first, but I may just merge it in soon.

Ghostbird · 2023-04-12T09:04:44Z

Sources/Microcharts.Forms/ChartView.cs

+
+                // e.Scale is the delta to be applied for the current frame
+                // Calculate the scale factor to be applied.
+                zoomCurScale += (eScale - 1) * zoomStartScale;


I haven't looked into this too much, but this seems overcomplicated.
It seems you're treating scaling as addition here, which means you need to do a -1 and treat it as an additive delta relative to a start. Scale operations are multiplicative. I made a very similar component once, and in my code I had a single line that said currentScale *= e.Scale. I think you can apply that logic to your code too. Treat e.Scale directly as a multiplicative delta and I think you can simplify.

brett-estabrook added 29 commits October 11, 2021 09:52

Merge pull request #1 from brett-estabrook/feature/36-null-value-support

0a86788

Adding test data

Initial commit of max value caching

fe9ab42

Cache maxvalue, minvalue, and valuerange to prevent constant recalculations during drawing loops

Cleanup Data and Logging

00782b9

Removing excess logging and adding better data generation

Updating min/max value handling

c9f8fde

Better test data. Fixing issue with user defined min/max values Adding an event for chart rendering complete

Fixing property changed events

5ea7ae9

Proper set comparison Don't double change animation progress when updating series

yAxisShift fix for bar

5725af3

The lines and bar charts were drawing too close to left labels

Adding some comments

72e41df

Adding some comments

Making the values match the labels

ede48dd

Making the values match the labels

Fixing issue with null values

2d6f4e5

Fixing issue with null values

Adding some comments about min/max values

e0f1c37

Adding some comments about min/max values

Merge branch 'feature/36-null-value-support' into feature/290-max-val…

257e712

…ue-perf-fixes

Merge branch 'feature/36-null-value-support' into feature/290-max-val…

9a570c7

…ue-perf-fixes

Merge branch 'feature/36-null-value-support' into feature/290-max-val…

695537c

…ue-perf-fixes

Revert "Merge branch 'feature/36-null-value-support' into feature/290…

744dda3

…-max-value-perf-fixes" This reverts commit 695537c, reversing changes made to 9a570c7.

Making labels match values

59659ab

Making labels match values

Initial support for ClipRect and Pinch to Zoom

db4d0e7

Initial support for ClipRect and Pinch to Zoom

Merge branch 'main' into feature/290-max-value-perf-fixes

5d932b1

Addressing an issue with line charts a indexing into draw points

f73549d

Addressing an issue with line charts a indexing into draw points

Merge branch 'feature/290-max-value-perf-fixes' into poc/min-value-cl…

223b2a7

…iprect

Handle scaling drawing of axis labels

f64c4c2

Handle scaling drawing of axis labels Improved clip rects

Fixing some PR related bugs

52dd9de

Applying correct equality check Moving label drawing into its own loop Fix label count in chart dynamic data

Merge branch 'feature/290-max-value-perf-fixes' into poc/min-value-cl…

583dd54

…iprect

Calculate height of Y Axis Labels and pad cliprect

f9d9e6a

Calculate height of Y Axis Labels and pad cliprect

Label cleanup, and option to disable zoom

3eddd6c

EnableZoom property, XAxisMaxLabels allows to not draw all labels when over dense data points Zoom on Y Axis ticks and X Axis labels

Fixing YAxis format on zoom

684c92c

Adjusting yAxis based on scale

5fd878f

Adding support for panning

1ad1196

Adding support for panning

Don't enable for every chart by default

0819d5d

Standardizing on precision

f6fd13a

brett-estabrook changed the title ~~Feature/296 pinch and zoom~~ Feature/296-pinch-and-zoom Nov 1, 2021

brett-estabrook commented Nov 1, 2021

View reviewed changes

brett-estabrook marked this pull request as ready for review November 1, 2021 00:35

eman1986 requested a review from Seuleuzeuh November 3, 2021 03:19

Ghostbird reviewed Apr 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/296-pinch-and-zoom #297

Feature/296-pinch-and-zoom #297

brett-estabrook commented Nov 1, 2021 •

edited

brett-estabrook Nov 1, 2021

eman1986 Nov 3, 2021

brett-estabrook Nov 3, 2021

brett-estabrook Nov 1, 2021

brett-estabrook Nov 1, 2021

brett-estabrook Nov 1, 2021

brett-estabrook Nov 1, 2021

brett-estabrook Nov 1, 2021

andrveld commented Feb 25, 2022

eman1986 commented Feb 25, 2022

Ghostbird Apr 12, 2023 •

edited

Feature/296-pinch-and-zoom #297

Are you sure you want to change the base?

Feature/296-pinch-and-zoom #297

Conversation

brett-estabrook commented Nov 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrveld commented Feb 25, 2022

eman1986 commented Feb 25, 2022

Ghostbird Apr 12, 2023 • edited

Choose a reason for hiding this comment

brett-estabrook commented Nov 1, 2021 •

edited

Ghostbird Apr 12, 2023 •

edited