chart-fx icon indicating copy to clipboard operation
chart-fx copied to clipboard

Major Grid lines sometimes don't update on DataSet updates that change the axis range of a chart

Open domenicquirl opened this issue 3 years ago • 3 comments

Describe the bug Plotting a small sine wave over a bit more than 10 seconds:

image

When I update the signal with data for up to 100 seconds, sometimes the previous grid lines remain and appear "squashed" at the beginning of the chart:

image

Similarly, when updating from 100 to 10+ seconds, sometimes the previous line distances remain and only one line is drawn:

image

Expected result for the 100 seconds, which sometimes I get automatically and can otherwise trigger by "updating" the data from 100 to 100 seconds manually:

image

To Reproduce The chart is just an XYChart with several plugins:

public SignalChart(SignalChartGroup group) {
	  super( new DefaultNumericAxis( "time", "s" ), new DefaultNumericAxis( "signal value", null ) );
	  this.group = group;
	  setLegendSide( Side.RIGHT );
	  getPlugins().add( new ParameterMeasurements() );
	  getPlugins().add( new DataPointTooltip() );
	  getPlugins().add( new Panner() );
	  getPlugins().add( new Screenshot() );
	  getPlugins().add( new ColormapSelector() );
	  
	  zoom = new Zoomer();
	  getPlugins().add( zoom );
}

public void updateDataSet( DataSet dataSet ) {
	  dataSetsByName.compute( dataSet.getName(), ( name, currentData ) -> {
	  if ( currentData == null ) {
		  getDatasets().add( dataSet );
		  return dataSet;
	  } else {
		  currentData.set( dataSet, false );
		  return currentData;
	  }
	  } );
	  Platform.runLater( () -> zoom.zoomOrigin() );
}

I tried setting the grid to invisible before and visible again after the update and firing an additional invalidated event for the chart after the update, both to no avail.

As the code shows I have the chart set up so it calls Zoomer::zoomOrigin on updates. When previously zoomed in, the misrendering still happens, but appears to happen less frequently. Together with the fact that I can manually re-trigger an update with the same dataset to restore the expected grid, this seems to suggest a timing or delay component of the error.

Environment:

  • OS: Windows, MacOS (colleague)
  • Java version: AdoptOpenJDK 15.0.2.7-hotspot
  • JavaFx version: OpenJFX 15.0.1
  • ChartFx version: chartfx-chart 11.2.2

domenicquirl avatar May 03 '21 09:05 domenicquirl

Hey,

welcome to chart-fx and thanks for the detailed and comprehensive bugreport. I've also noticed this sporadically, but I've never come around to analyzing it systematically.

Could you try "chart.getGridRenderer().setDrawOnTop(true)" ? That renders the grid after the chart on top of it, so it could help as a workaround.

But from looking at the code I think the tick marks are only updated during rendering and and it seems the canvas is rendered before the axis. Probably we have to invalidate the chart canvas whenever the tick marks have been recomputed or calll recompute tick marks before rendering the Grid. The first could be accomplished ad-hoc by with by overriding AbstractAxis's default empty tickMarksUpdated() to request an a chart layout (would lead to rendering the chart twice). This could then just be added to the layoutChildren method in AbstractAxis. For the latter I don't know if it is safe to call the Axis's layoutChildren() from within the GridRenderer's render() method.

This is all I can think of without getting my hands dirty for now. I'll try to find the time to find a proper fix for this. If you find something first, don't hesitate to share your findings here or add a PR with a fix.

wirew0rm avatar May 03 '21 18:05 wirew0rm

Again, thanks for your prompt response.

I tested your suggestions where actionable. Changing the grid renderer to setDrawOnTop(true) does not resolve the issue, instead the wrong grid lines are rendered over the plotted series.

For overriding axis methods, I found the following:

protected void tickMarksUpdated() {
	// do not resolve the issue
	baseChart.fireInvalidated()
	// --- OR ---
	Platform.runLater( () -> baseChart.fireInvalidated() );
	
	// resolve the issue
	baseChart.requestLayout()
	// --- OR ---
	baseChart.getGridRenderer().getHorizontalMajorGrid().setVisible( false );
	baseChart.getGridRenderer().getHorizontalMajorGrid().setVisible( true );
}

Since this is not too much effort, the override is a sufficient work-around for me to keep working. If I understand you correctly, this would however not be the preferred fix due to the double rendering of the chart?

domenicquirl avatar May 03 '21 18:05 domenicquirl

Thanks for confirming/testing and I hope this will allow you to work around the issue until the next release.

wirew0rm avatar May 03 '21 19:05 wirew0rm

Fixed by #592, hope to do a release including the changes out soon. Closing for now to have a better overview on outstanding issues before the release, feel free to reopen if the issue persists on main/the next release.

wirew0rm avatar Aug 09 '23 08:08 wirew0rm