Bug or feature? Combining ErrorDataSetRenderer and XYChart

[wolfig]

Describe the bug
I add a XYChart to a JavaFX GridPane. The XYChart has one additional renderer, an ErrorDataSetRenderer. Before adding the XYChart to the GridPane, the XYChart has two axes: one x-axis and one y-axis.

The XYChart is initialized like this:

private XYChart _xyChart = new XYChart(_xAxis, _yAxis);

public DeviceAutomatorXYChart() {
    _renderer = new ErrorDataSetRenderer();
    _renderer.setId("MainRenderer");
    _renderer.setErrorStyle(ErrorStyle.ERRORBARS);
    _renderer.setDrawMarker(true);
    _renderer.setShowInLegend(false);
    _xyChart.setLegendVisible(false);
    _xyChart.getRenderers().clear();
    _xyChart.getRenderers().add(_renderer);
 }

During the GridPane.add process, the method XYChart.runPreLayout() is triggered, which calls (Chart.class line 490f)

for (Renderer renderer : renderers) {
    renderer.updateAxes();
}

The "updateAxes" calls AbstractRendererXY.class lines 75f.:

@Override
public void updateAxes() {
    // Default to explicitly set axes
    xAxis = ensureAxisInChart(getFirstAxis(Orientation.HORIZONTAL));
    yAxis = ensureAxisInChart(getFirstAxis(Orientation.VERTICAL));

    // Get or create one in the chart if needed
    var chart = AssertUtils.notNull("chart", getChart());
    if (xAxis == null) {
        xAxis = chart.getXAxis();
    }
    if (yAxis == null) {
        yAxis = chart.getYAxis();
    }

"ensureAxisInChart" AbstractRendererXY.class lines 100f.:

protected Axis ensureAxisInChart(Axis axis) {
    if (axis != null && !getChart().getAxes().contains(axis)) {
        getChart().getAxes().add(axis);
    }
    return axis;
}

In case of the x-axis, axis is not null, for the y-axis axis is null. getChart().getAxes().contains(axis) yields false for the x-axis and the axis from the ErrorDataSetRenderer is added to the chart.
Cause of this behavior is that the axes in the instance of ErrorDataSetRenderer are different instances than those owned by the XYChart.

I can fix this behavior by explicitly adding _renderer.getAxes().setAll(_xyChart.getAxes());, but this seems to me a bit awkward - shouldn't the default behavior be that the renderer and the chart share the axes and the API user adds them explicitly if desired?

[wirew0rm]

Thanks for the detailed description, this definitely seems like an unexpected behavior.

What I'm wondering is why the renderer has an x-axis at all before it is added to the chart. I suspect that one of the properties you modify cause a call to getXAxis() and because the renderer is not yet added to a chart it is forced to create a new one.

shouldn't the default behavior be that the renderer and the chart share the axes and the API user adds them explicitly if desired?
Yes, and usually this is what the code does, the problem here is that something causes the Renderer to already have an x axis before even being added to the chart.
To find the root of this could you step through your DeviceAutomatorXYChart(...) function and check after each line if renderer.axesList already contains the xAxis?

In terms of fixes, just clearing the renderer's axes should be sufficient, as then it will get the corresponding axes from the chart on first use.

In general I prefer to do foo.setAll(bar) over foo.clear(); foo.add(bar), since it will prevent a lot of binding evaulations with incomplete lists. (I don't think it's the problem in this case and even if it were it should still work, of course.)

Also just to make sure, I assume you are on the latest released version? One of that functions was changed recently, but I don't think it would affect this problem.