Feature/296-pinch-and-zoom #297
Conversation
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
Making labels match values
Initial support for ClipRect and Pinch to Zoom
Addressing an issue with line charts a indexing into draw points
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
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
| /// </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(); |
There was a problem hiding this comment.
Not sure if this should be public or not, I could see renaming EnableZoom to EnableXForm and allowing users to do what they want...
There was a problem hiding this comment.
what benefit would this give the user? I'm thinking it could be private
There was a problem hiding this comment.
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.
| /// <summary> | ||
| /// How many labels to draw, -1 means all of them | ||
| /// </summary> | ||
| public int XAxisMaxLabels { get; set; } = -1; |
There was a problem hiding this comment.
I added this for densely packed points. So we can reduce the draw on the horizontal axis. -1 will draw all labels by default.
| 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); |
There was a problem hiding this comment.
I calculate a NiceMin/NiceMax once for a scale of 1.0 then use that as a base when recalculating the yAxis while zoomed
|
|
||
| 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)); |
There was a problem hiding this comment.
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.
| if( axisChart != null && axisChart.EnableZoom ) | ||
| { | ||
| //FIXME: how to handle disable zoom after already enabled | ||
| var pinchGesture = new PinchGestureRecognizer(); |
There was a problem hiding this comment.
I wonder if this should be moved into the AxisBasedChart and then have the ChartView call an init function on the chart.
|
|
||
| float yAxisXShift; | ||
| List<float> yAxisIntervalLabels; | ||
| string yAxisFormat = XForm.Scale == 1.0f ? "G" : "F1"; //As we scale the yAxis we get more decimal places |
There was a problem hiding this comment.
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.
|
Are there plans for this to be merged soon? |
|
I was trying to let another person on the team review this first, but I may just merge it in soon. |
|
|
||
| // e.Scale is the delta to be applied for the current frame | ||
| // Calculate the scale factor to be applied. | ||
| zoomCurScale += (eScale - 1) * zoomStartScale; |
There was a problem hiding this comment.
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.
|
Is this repository de-facto dead? |
While I'm no longer doing xamarin development, I do welcome pull requests. This one has been under work, if someone wants to take this to the finish line I'd appreciate it. |
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
Charts tested:
Platform tested :
Fix: