Skip to content

Polar Axis: convert from an IPlottable to an IGrid#4152

Closed
CoderPM2011 wants to merge 3 commits intoScottPlot:mainfrom
CoderPM2011:PolarAxisByIGrid
Closed

Polar Axis: convert from an IPlottable to an IGrid#4152
CoderPM2011 wants to merge 3 commits intoScottPlot:mainfrom
CoderPM2011:PolarAxisByIGrid

Conversation

@CoderPM2011
Copy link
Contributor

@CoderPM2011 CoderPM2011 commented Aug 13, 2024

resolve #4105

In this PR, I have adjusted certain interfaces to address the rendering area and zoom issues. However, I am not completely satisfied with these modifications, as they were made directly based on the needs of the Polar Axis without extensive consideration.

The key issue lies in the Polar Axis, where the rendering restrictions do not necessarily align with the IPlottable interface. To achieve a more consistent and accurate rendering, it may be necessary for the IGrid interface to participate in the chart rendering or data range decision-making process, similar to the IPlottable interface.

result image

image

sample code

ScottPlot.Plot myPlot = new();
myPlot.Axes.Rules.Add(new SquareZoomOut(
    myPlot.Axes.Bottom, myPlot.Axes.Left));
PolarAxis polarAxis = new(
    formsPlot1.Plot.Axes.DefaultGrid.XAxis,
    formsPlot1.Plot.Axes.DefaultGrid.YAxis)
{
    MaximumRadius = 30,
};
polarAxis.RegenerateCircles(count: 10);
polarAxis.RegenerateSpokes(count: 4);
myPlot.Axes.CustomGrids.Add(polarAxis);

var points = new PolarCoordinates[] {
    new(10, Angle.FromDegrees(15)),
    new(20, Angle.FromDegrees(120)),
    new(30, Angle.FromDegrees(240)),
};

IPalette palette = new ScottPlot.Palettes.Category10();
Coordinates center = polarAxis.GetCoordinates(0, 0);
for (int i = 0; i < points.Length; i++)
{
    Coordinates tip = polarAxis.GetCoordinates(points[i]);
    var arrow = myPlot.Add.Arrow(center, tip);
    arrow.ArrowLineWidth = 0;
    arrow.ArrowFillColor = palette.GetColor(i).WithAlpha(.7);
}

@swharden
Copy link
Member

swharden commented Aug 25, 2024

Hi @CoderPM2011, thanks for this PR! It seems to do a lot more than fix the default plottables colors as described in #4105 😅

I also see the code in this PR doesn't actually build. I probably won't take time to figure out how to get it to build before trying it out.

I see this PR makes PolarAxis an IGrid, but to be honest I'm pretty hesitant to do this. The polar axis is really just a class that knows how to draw circles on a Cartesian plane, and translate polar coordinates to Cartesian space. I don't really consider it a grid itself, but because of its naming I can see where this may be confusing.

Recognizing the core issue of wanting to improve autoscaling behavior, is it possible that we can move scaling behavior for polar axes out of the FractionalAutoscaler and put it in a function inside the PolarAxis itself? I haven't thought through what the API would look like in detail, but I'm curious about your thoughts about feasibility of this idea and what a good name may be.

Thanks for your work on this PR, and thanks for your thoughts on my suggestion as well!

Scott

@swharden swharden mentioned this pull request Aug 26, 2024
Merged
@swharden
Copy link
Member

swharden commented Aug 26, 2024 •
edited
Loading

It seems to do a lot more than fix the default plottables colors as described in #4105

Following-up, I recognize that I titled #4105 poorly 😅

The issue describes the problem (the color thing), but the title suggests an implementation (making IPlottable an IGrid). I wanted to voice that @CoderPM2011 did a good job applying the implementation suggestion in that title, and that it was my fault that there was probably a simpler way to achieve that core functionality.

I'm merging in #4175 which I think solves the color issue. I think this PR may still be worth building out (to improve autoscaling behavior?), but it no longer needs to solve the color issue 👍

Sorry for that confusion!

@CoderPM2011
Copy link
Contributor Author

CoderPM2011 commented Aug 26, 2024

Following-up, I recognize that I titled #4105 poorly 😅

The issue describes the problem (the color thing), but the title suggests an implementation (making IPlottable an IGrid). I wanted to voice that @CoderPM2011 did a good job applying the implementation suggestion in that title, and that it was my fault that there was probably a simpler way to achieve that core functionality.

I'm merging in #4175 which I think solves the color issue. I think this PR may still be worth building out (to improve autoscaling behavior?), but it no longer needs to solve the color issue 👍

Sorry for that confusion!

It's okay. 😉

While I was trying to fix the Cookbook, I discovered that this PR has other issues as well.
For example, when there is no data and the main grid is hidden, the rendering area of the PolarAxis also becomes incorrect. 😓
If we really want to change the PolarAxis to the IGrid type, the adjustments required might be more extensive than we initially thought. 🤔

@swharden
Copy link
Member

swharden commented Aug 26, 2024

It's okay. 😉

I'm glad you understand!

I'll close this PR assuming things are all right where they landed, and if you decide to implement some functionality like autoscaling let's pick up that topic in a new PR 😎

@swharden swharden closed this Aug 26, 2024
@CoderPM2011 CoderPM2011 deleted the PolarAxisByIGrid branch September 2, 2024 12:46
@mysteryname mysteryname mentioned this pull request Jan 10, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Polar Axis: convert from an IPlottable to an IGrid

2 participants

@CoderPM2011 @swharden