Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tick mark label sizes are incorrectly calculated resulting in a poor layout #699

Closed
MisterRedactus opened this issue Jan 17, 2021 · 13 comments
Labels
BUG unexpected behavior

Comments

@MisterRedactus
Copy link

I am having an issue with a plot cast as SignalPlotXY. Everything works well, but the Y-Axis label is on top of the Y-Axis tick labels, which are about 6-7 places in length. Is this a gap in my understanding or a bug, and, if the former, how do I address this?

@MisterRedactus MisterRedactus added the BUG unexpected behavior label Jan 17, 2021
@bclehmann
Copy link
Member

Hello, since it's a visual issue a screenshot would help a lot in understanding or diagnosing your issue. If possible, the relevant code would also be a plus.

If you're plotting sensitive data then a screenshot of just the affected axis would be fine, or an example with the same issue but meaningless data.

@MisterRedactus
Copy link
Author

Certainly. My original code was part of a much larger app and would have been difficult to easily summarize. However, I have been able to duplicate the issue in a much smaller app. My xaml:

<Window x:Class="SagacityPipeline.Designer.Windows.SequentialChartWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:local="clr-namespace:SagacityPipeline.Designer.Windows" xmlns:ScottPlot="clr-namespace:ScottPlot;assembly=ScottPlot.WPF" mc:Ignorable="d" Title="SequentialChartWindow" Height="450" Width="800"> <Grid> <Grid.RowDefinitions> <RowDefinition Height="1*" /> <RowDefinition Height="10*" /> </Grid.RowDefinitions> <Button Grid.Row="0" Click="ShowLabel" >Button 1</Button> <ScottPlot:WpfPlot Name="wpfPlot1" Grid.Row="1"/> </Grid> </Window>

Code behind:

        // generate random, unevenly-spaced data
        Random rand = new Random(1);
        int pointCount = 500_000;
        double[] ys = new double[pointCount];
        double[] xs = new double[pointCount];

        ys[0] = 50000;
        for (int i = 1; i < ys.Length; i++)
        {
            ys[i] = ys[i - 1] + rand.NextDouble() - .5;
            xs[i] = xs[i - 1] + rand.NextDouble();
        }

        wpfPlot1.plt.YAxis.Label("Test Label");
        wpfPlot1.plt.AddSignalXY(xs, ys);

Screenshot:

image

@MisterRedactus
Copy link
Author

I should probably also mention that I am using version 4.1.3-beta.

@bclehmann
Copy link
Member

Thank you, for the time being using wpfPlot1.plt.Layout() might help until this gets fixed.

The arguments are all optional, you can set only left padding like this: wpfPlot1.plt.Layout(leftPadding).

Here is the code for Plot.Layout for reference:

https://github.com/ScottPlot/ScottPlot/blob/master/src/ScottPlot/Plot/Plot.Axis.cs#L102-L111

@MisterRedactus
Copy link
Author

Thanks! I can confirm that setting wpfPlot1.plt.Layout(left: 65); appears to provide a reasonable separation between ticks and left axis label in the sample app. That said, I am wondering:

  1. What happens if you have multiple left (or right) axes? It would seem to me that padding control needs to be implemented at the individual axis level.
  2. Is it possible to make the separation distance between axis label and axis ticks an automatic function? Alternately, the padding could somehow be referenced to the outer edge of the axis.

This issue aside, this is an impressive application. Kudos to all who have been working on it.

@bclehmann
Copy link
Member

  • What happens if you have multiple left (or right) axes? It would seem to me that padding control needs to be implemented at the individual axis level.

I don't think I know what you mean, I don't know how you would have two left axes.

  • Is it possible to make the separation distance between axis label and axis ticks an automatic function? Alternately, the padding could somehow be referenced to the outer edge of the axis.

Yes, this should probably use Graphics.MeasureString, the only thing is it would need to accommodate rotated axis labels, which just means some simple trigonometry.

@MisterRedactus
Copy link
Author

With respect to the second issue, I agree that this is a fairly easily solved trig problem.

But with respect to the first issue, however, ScottPlot has the ability to add additional Y-Axes via the plt.AddAxis() method. This stacks additional vertical axes away from the plot area on either the left or right side, depending on how you initialize the axis. This is discussed in the 4.1.3-beta cookbook. For example, in the cookbook example, the line of code var yAxis3 = plt.AddAxis(Renderable.Edge.Left, axisIndex: 2); stacks a new Y-Axis on the left side of the plot and assigns it an index of 2, since presumably the automatically created left and right Y-Axes are already assigned indices 0 and 1. The axis to be used for the SignalXY object/series is then specified by the SignalPlotXY.YAxisIndex property. This is a pretty cool feature, and I have been using it extensively. My point is that if every axis has a problem with the axis label overlapping the ticks, then this issue needs to be solved at the Y-Axis object level.

@swharden
Copy link
Member

swharden commented Jan 23, 2021

Hi @MisterRedactus, thank you so much for reporting this!

The axis sizes should be frequently recalculated to accommodate the ticks and labels. It looks like this is no longer happening, and the simplest way to demonstrate this is to zoom out on some of the cookbook examples:
image

I think this problem sneaked in when I refactored all of the user controls in the last couple weeks (#683). I'll take a look now and am optimistic I can fix this today...

EDIT: This broke between versions 4.1.2 and 4.1.3

@swharden swharden changed the title SignalXY Plot Axis Label Overlaps Tick Labels User controls do not properly recalculate layout Jan 23, 2021
@MisterRedactus
Copy link
Author

Excellent! I'll be looking forward to your latest update. And again, I need to mention that this is a terrific app. You've done a lot of great work!

@swharden
Copy link
Member

Thanks for the encouragement @MisterRedactus!

I figured out that the problem is caused by these two lines added in 2cdeb10 which attempted to improve the calculation of label dimensions for rotated ticks. I'm guessing the formula is wrong (or perhaps is only right for horizontal, but not vertical axes).

For an immediate fix, comment-out these two lines:

float diff = Math.Max(maxWidth, maxHeight) - Math.Min(maxWidth, maxHeight);
sizeNeeded = Math.Min(maxWidth, maxHeight) + diff * (float)Math.Sin(AxisTicks.TickLabelRotation * Math.PI / 180);

@swharden swharden changed the title User controls do not properly recalculate layout Tick mark label sizes are incorrectly calculated resulting in a poor layout Jan 23, 2021
@bclehmann
Copy link
Member

bclehmann commented Jan 23, 2021

@swharden The issue is that the formula is only correct for horizontal axes, the problem is the use of the sine ratio, sine will give the vertical displacement, which only matters on the horizontal axis. The solution is to have an if statement that checks which edge it is on and uses sine or cosine based on that. Although since vertical axes don't currently support tick label rotation (and cos 0 = 1) you don't actually need trig.

In #706 I addressed this, my code looked like this:

switch (Edge)
{
	case Edge.Top:
	case Edge.Bottom:
		sizeNeeded = Math.Min(maxWidth, maxHeight) + diff * (float)Math.Sin(AxisTicks.TickLabelRotation * Math.PI / 180);
		break;

	case Edge.Left:
	case Edge.Right:
		sizeNeeded = Math.Min(maxWidth, maxHeight) + diff * (float)Math.Cos(AxisTicks.TickLabelRotation * Math.PI / 180);
		break;
}

Feel free to adapt my solution if you'd like, I know not everyone is a fan of fall-through switch statements.

@swharden
Copy link
Member

@Benny121221, thanks for pointing that out! I've been going through issues bottom-to-top and I hadn't looked deep into #706 yet, and I now see that you figured out this error and offered a solution. Thanks so much!

I think I'll move toward all axes supporting all rotations. Previously only the lower X axis supported it, but this was because of laziness. I'll modify #706 as needed to try to support all axes and merge it in. Thanks for solving @MisterRedactus problem too!

swharden added a commit to bclehmann/ScottPlot that referenced this issue Jan 24, 2021
Additional refactoring is still required (see TODO comment)

Discussion in ScottPlot#706 and ScottPlot#699
bclehmann pushed a commit to bclehmann/ScottPlot that referenced this issue Jan 24, 2021
Additional refactoring is still required (see TODO comment)

Discussion in ScottPlot#706 and ScottPlot#699
@swharden
Copy link
Member

This has been fixed by @Benny121221 in #708. In addition to the fix, rotated vertical axis tick labels are now supported. I'll publish a package on NuGet later today.

Thanks for pointing-out this bug @MisterRedactus! I'm glad to see it's fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG unexpected behavior
Projects
None yet
Development

No branches or pull requests

3 participants