fair-acc / chart-fx

A scientific charting library focused on performance optimised real-time data visualisation at 25 Hz update rates for data sets with a few 10 thousand up to 5 million data points.
GNU Lesser General Public License v3.0
504 stars 93 forks source link

Parameter Measurement does not work with long named dataset #564

Closed flef closed 1 year ago

flef commented 1 year ago

Describe the bug When asking to compute a Measurement (for example LowPass filter) on a dataset with a long name (> 17 characters), the result box will become bigger and bigger taking the full screen hiding the chart. (as long as you move your mouse on it).

To Reproduce The problem can be reproduced by asking UI to produce

 final XYChart chart = new XYChart(new DefaultNumericAxis(), yAxis);
        chart.getPlugins().addAll(new ParameterMeasurements());
(...)

    final DoubleDataSet dataSetWorking = new DoubleDataSet("data set with max");
    final DoubleDataSet dataSetNotWorking = new DoubleDataSet("data set with long name that will produce error");

Environment:

flef commented 1 year ago

In pictures :

Capture d’écran du 2023-01-05 20-42-03

Then just by mouving the mouse over

Capture d’écran du 2023-01-05 20-46-39

wirew0rm commented 1 year ago

Hey thank you for the report. I didn't have time to reproduce and the debug the issue but I would suspect that the issue is with this function automatically scaling the textboxes: https://github.com/fair-acc/chart-fx/blob/main/chartfx-chart/src/main/java/io/fair_acc/chartfx/plugins/measurements/utils/CheckedValueField.java#L125 Maybe we should add a check there, which only increases the font size if this doesn't increase the size of the panel. Please feel free to ask further questions and share your findings/questions/fixes/PRs :) I'll try to look into it, but I cannot make any promises.

flef commented 1 year ago

Hey,

That's it ! If you remove this line everything works fine after. Just testing via Reflection. I think there is no need to change the font size.

Really sorry for the dirty piece of code :

var pm = new ParameterMeasurements(Side.RIGHT, true);
pm.getChartMeasurements().addListener((ListChangeListener<? super AbstractChartMeasurement>) change -> {
    while (change.next()) {
        if (change.wasAdded()) {
            change.getAddedSubList().forEach(a -> {
                try {
                    Field widthPropertyField = a.getValueField().getClass()
                            .getDeclaredField("widthChangeListener");
                    System.err.println(widthPropertyField);
                    widthPropertyField.setAccessible(true);

                                        // Here remove the suspected listener
                    a.getValueField().widthProperty().removeListener(
                            (ChangeListener<? super Number>) widthPropertyField.get(a.getValueField()));
                } catch (Exception e) {
                    System.err.println(e);
                }
            });
        }
    }
});

final XYChart chart = new XYChart(new DefaultNumericAxis(), yAxis);
chart.getPlugins().addAll(new Zoomer(), new CrosshairIndicator(), pm, new EditAxis());

Final result : Capture d’écran du 2023-01-10 22-17-17

wirew0rm commented 1 year ago

I've disabled the font size adjustment for now, can be revisited later if it can be reenabled with some guardrails against growing to infinity.