-
Notifications
You must be signed in to change notification settings - Fork 111
Bugfix/705 zoomer plugin issues with inverted axes #706
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
base: main
Are you sure you want to change the base?
Changes from all commits
dfc2665
ea65748
7e8ec37
7cda3b7
0415937
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,7 @@ | |
| import io.fair_acc.chartfx.XYChart; | ||
| import io.fair_acc.chartfx.axes.Axis; | ||
| import io.fair_acc.chartfx.axes.AxisMode; | ||
| import io.fair_acc.chartfx.axes.spi.DefaultNumericAxis; | ||
| import io.fair_acc.chartfx.ui.ObservableDeque; | ||
| import io.fair_acc.chartfx.ui.geometry.Side; | ||
|
|
||
|
|
@@ -893,8 +894,18 @@ private void panChart(final Chart chart, final Point2D mouseLocation) { | |
| final double offset = (side.isHorizontal() ? oldMouseX : oldMouseY) - (side.isHorizontal() ? newMouseX : newMouseY); | ||
| axis.setAutoRanging(false); | ||
| // shift bounds | ||
| final double newMin = axis.getValueForDisplay(0 + offset); | ||
| final double newMax = axis.getValueForDisplay(axis.getLength() + offset); | ||
| final double newDisplayPositionMin; | ||
| final double newDisplayPositionMax; | ||
| if (axis.isInvertedAxis()) { | ||
| newDisplayPositionMin = axis.getLength() + offset; | ||
| newDisplayPositionMax = 0 + offset; | ||
| } else { | ||
| newDisplayPositionMin = 0 + offset; | ||
| newDisplayPositionMax = axis.getLength() + offset; | ||
| } | ||
|
|
||
| final double newMin = axis.getValueForDisplay(newDisplayPositionMin); | ||
| final double newMax = axis.getValueForDisplay(newDisplayPositionMax); | ||
| if (side.isHorizontal()) { | ||
| axis.set(newMin, newMax); | ||
| } else { | ||
|
|
@@ -1200,9 +1211,20 @@ private static void zoomOnAxis(final Axis axis, final ScrollEvent event) { | |
| maxDisplay = (1 - scaling) * mousePos; | ||
| minDisplay = mousePos + scaling * (max - mousePos); | ||
| } | ||
| final double newMin = axis.getValueForDisplay(minDisplay); | ||
| final double newMax = axis.getValueForDisplay(maxDisplay); | ||
|
|
||
|
|
||
| final double minDisplayWithInverted; | ||
| final double maxDisplayWithInverted; | ||
| if (axis.isInvertedAxis()) { | ||
| minDisplayWithInverted = maxDisplay; | ||
| maxDisplayWithInverted = minDisplay; | ||
| } else { | ||
| minDisplayWithInverted = minDisplay; | ||
| maxDisplayWithInverted = maxDisplay; | ||
| } | ||
|
|
||
| final double newMin = axis.getValueForDisplay(minDisplayWithInverted); | ||
| final double newMax = axis.getValueForDisplay(maxDisplayWithInverted); | ||
|
|
||
| axis.set(newMin, newMax); | ||
| axis.updateCachedTransforms(); | ||
|
|
||
|
|
@@ -1333,8 +1355,9 @@ public ZoomRangeSlider(final Chart chart) { | |
| setPrefWidth(-1); | ||
| setMaxWidth(Double.MAX_VALUE); | ||
|
|
||
| xAxis.invertAxisProperty().bindBidirectional(invertedSlide); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This did set Bidirectional binding was not required here IMO. |
||
| invertedSlide.addListener((ch, o, n) -> setRotate(Boolean.TRUE.equals(n) ? 180 : 0)); | ||
| invertedSlide.set(xAxis.isInvertedAxis()); | ||
| invertedSlide.bind(xAxis.invertAxisProperty()); | ||
|
|
||
| xAxis.autoRangingProperty().addListener(sliderResetHandler); | ||
| xAxis.autoGrowRangingProperty().addListener(sliderResetHandler); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package io.fair_acc.chartfx.plugins; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.testfx.api.FxRobot; | ||
| import org.testfx.framework.junit5.ApplicationExtension; | ||
| import org.testfx.framework.junit5.Start; | ||
|
|
||
| import io.fair_acc.chartfx.XYChart; | ||
| import io.fair_acc.chartfx.axes.Axis; | ||
| import io.fair_acc.chartfx.axes.spi.DefaultNumericAxis; | ||
| import javafx.geometry.VerticalDirection; | ||
| import javafx.scene.Scene; | ||
| import javafx.stage.Stage; | ||
|
|
||
| @ExtendWith(ApplicationExtension.class) | ||
| public class ZoomerWithAxisInversionTest { | ||
|
|
||
| private XYChart chart; | ||
| private Axis xAxis; | ||
| private Axis yAxis; | ||
| private Zoomer zoomer; | ||
|
|
||
| @Start | ||
| void start(Stage stage) { | ||
| xAxis = new DefaultNumericAxis("x", 0, 1, 10); | ||
| yAxis = new DefaultNumericAxis("y", 0, 1, 10); | ||
| chart = new XYChart(xAxis, yAxis); | ||
| zoomer = new Zoomer(); | ||
| chart.getPlugins().add(zoomer); | ||
|
|
||
| stage.setScene(new Scene(chart)); | ||
| stage.show(); | ||
| } | ||
|
|
||
| // Test case for issue #705 | ||
| @Test | ||
| void testZoomWithInvertedAxis(FxRobot robot) { | ||
| yAxis.invertAxis(true); | ||
| robot.moveTo(chart); | ||
|
|
||
| assertThat(yAxis.getMin()).isLessThan(yAxis.getMax()); | ||
| robot.scroll(VerticalDirection.UP); | ||
|
|
||
| // Initially min = 0; max = 1. This is not changed by inverting the axis. | ||
| assertThat(yAxis.getMin()).as("y axis min and max exchanged").isLessThan(yAxis.getMax()); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
|
|
||
| import javafx.application.Application; | ||
| import javafx.collections.ListChangeListener; | ||
| import javafx.geometry.Orientation; | ||
| import javafx.geometry.Pos; | ||
| import javafx.scene.Node; | ||
| import javafx.scene.control.ContentDisplay; | ||
|
|
@@ -82,7 +83,15 @@ public Node getChartPanel(final Stage primaryStage) { | |
| registerZoomerChangeListener(zoomer5, chart5.getTitle()); | ||
| chart5.getPlugins().add(zoomer5); | ||
|
|
||
| root.getChildren().addAll(chart1, chart2, chart3, chart4, chart5, label); | ||
| // chart with inverted axes | ||
| final Chart chart6 = getTestChart("inverted axes", testDataSet); | ||
| chart6.getFirstAxis(Orientation.HORIZONTAL).invertAxis(true); | ||
| chart6.getFirstAxis(Orientation.VERTICAL).invertAxis(true); | ||
| final Zoomer zoomer6 = new Zoomer(); | ||
| registerZoomerChangeListener(zoomer6, chart6.getTitle()); | ||
| chart6.getPlugins().add(zoomer6); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For reproduction according to Jonas' report. |
||
|
|
||
| root.getChildren().addAll(chart1, chart2, chart3, chart4, chart5, chart6, label); | ||
|
|
||
| return root; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a left over of the bigger changes lately.
axisLengthfrom line 606 is already populated during initial rendering whilegetWidth()seems to be zero and causes the inverted axis to be not placed properly.