Skip to content

PlotDimensions: fix GetCoordinateY(), add tests, and refactor#1626

Merged
swharden merged 5 commits intomasterfrom
dims-tests
Feb 1, 2022
Merged

PlotDimensions: fix GetCoordinateY(), add tests, and refactor#1626
swharden merged 5 commits intomasterfrom
dims-tests

Conversation

@swharden
Copy link
Member

@swharden swharden commented Feb 1, 2022

resolves #1625

@swharden
Copy link
Member Author

swharden commented Feb 1, 2022 •
edited
Loading

Previously this was overlooked because the Plot module performs pixel/unit conversions like this

ScottPlot/src/ScottPlot/Renderable/AxisDimensions.cs

Lines 279 to 288 in 41d6cbb

/// <summary>
/// Get the pixel location on the figure for a given position in axis units
/// </summary>
public float GetPixel(double unit)
{
double unitsFromMin = IsInverted ? Max - unit : unit - Min;
double pxFromMin = unitsFromMin * PxPerUnit;
double pixel = DataOffsetPx + pxFromMin;
return (float)pixel;
}

ScottPlot/src/ScottPlot/Renderable/AxisDimensions.cs

Lines 290 to 297 in 41d6cbb

/// <summary>
/// Get the axis unit position for the given pixel location on the figure
/// </summary>
public double GetUnit(float pixel)
{
double pxFromMin = IsInverted ? DataSizePx + DataOffsetPx - pixel : pixel - DataOffsetPx;
return pxFromMin * UnitsPerPx + Min;
}

Perhaps some of this duplicated logic could be removed

EDIT: there is indeed extreme logic and state duplication between AxisDimensions and PlotDimensions. This is the result of an evolving back-end that grew to support multi-axis dimension systems. I won't fix it in ScottPlot 4, but will design this better in ScottPlot 5.

@swharden swharden merged commit a1c8432 into master Feb 1, 2022
@swharden swharden deleted the dims-tests branch February 1, 2022 00:32
@swharden swharden mentioned this pull request Feb 1, 2022
Merged
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.

Axis: inaccurate GetCoordinateY() conversion

1 participant

@swharden