Skip to content
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

Roadmap to 11.3 #527

Closed
18 of 27 tasks
wirew0rm opened this issue Jun 28, 2022 · 12 comments
Closed
18 of 27 tasks

Roadmap to 11.3 #527

wirew0rm opened this issue Jun 28, 2022 · 12 comments
Labels
Milestone

Comments

@wirew0rm
Copy link
Member

wirew0rm commented Jun 28, 2022

This is the roadmap issue collecting issues potentially breaking API, which should be addressed for the 11.3 release.

This announcement is here to track progress, but also to collect feedback from the community and to catch potential problems for consumers of the library early on.

@brainbytes42
Copy link

brainbytes42 commented Sep 6, 2022

The library sounds promising - getting it was a little confusing at first. The readme has already the new maven-target ('io.fair-acc:chartfx:11.3.0'), which isn't released yet. The currently released version ('de.gsi:chartfx:11.2.7' 'de.gsi.chart:chartfx-chart:11.2.7') would be more helpful there... ;-)

@crowlogic
Copy link

Exception in Application start method
java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:119)
at java.base/java.lang.reflect.Method.invoke(Method.java:577)
at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplicationWithArgs(LauncherImpl.java:464)
at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication(LauncherImpl.java:363)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:577)
at java.base/sun.launcher.LauncherHelper$FXHelper.main(LauncherHelper.java:1081)
Caused by: java.lang.RuntimeException: Exception in Application start method
at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:900)
at javafx.graphics/com.sun.javafx.application.LauncherImpl.lambda$launchApplication$2(LauncherImpl.java:195)
at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NoSuchMethodError: 'void javafx.scene.canvas.GraphicsContext.setImageSmoothing(boolean)'
at de.gsi.chartfx.chart@11.2.7/de.gsi.chart.renderer.spi.ContourDataSetRenderer.drawHeatMap(ContourDataSetRenderer.java:200)
at de.gsi.chartfx.chart@11.2.7/de.gsi.chart.renderer.spi.ContourDataSetRenderer.paintCanvas(ContourDataSetRenderer.java:383)
at de.gsi.chartfx.chart@11.2.7/de.gsi.chart.renderer.spi.ContourDataSetRenderer.render(ContourDataSetRenderer.java:433)
at de.gsi.chartfx.chart@11.2.7/de.gsi.chart.XYChart.redrawCanvas(XYChart.java:433)
at de.gsi.chartfx.chart@11.2.7/de.gsi.chart.Chart.layoutChildren(Chart.java:756)

@crowlogic
Copy link

The library sounds promising - getting it was a little confusing at first. The readme has already the new maven-target ('io.fair-acc:chartfx:11.3.0'), which isn't released yet. The currently released version ('de.gsi:chartfx:11.2.7' 'de.gsi.chart:chartfx-chart:11.2.7') would be more helpful there... ;-)

You are right of course but apparently the maintainers of this project are doing other things or got stampeded by a herd of buffalo or something

@wirew0rm
Copy link
Member Author

wirew0rm commented Sep 26, 2023

Since the major points on the 11.3 milestone have now been addressed, the current state of the main branch will become the 11.3 release in the next days.

It brings lots of (mostly internal) reworking that massively improves performance, reliability and maintainability, lot of the hard work there was done by ennerf (Florian Enner)
. You can take a look at his PRs which include measurements and before/after comparisons. These improvements also come with very little changes to existing user code.

The biggest user facing change is the move from the de.gsi.chart* package name to io.fair_acc.chart-fx.*. While it is a bit annoying it should be mostly mechanical renames.

You can already test the main branch with the snapshot release as described in the README.md or wait for the imminent release. Let us know if you encounter any problems.

@protogenes
Copy link
Contributor

Please make it possible to adjust line join and line cap for renderers and/or change the default values.
When rendering data as a polyline with very acute angles the joins are drawn with very large miters (easily above 30px) and that really distorts the chart.

@ennerf
Copy link
Collaborator

ennerf commented Sep 28, 2023

@protogenes Can you provide an example and screenshot that show the problem with the default settings?

The default renderers don't set those values, so as an initial workaround you should be able to globally modify the defaults, e.g.,

var chart = new XYChart(xAxis, yAxis);
var gc= chart.getCanvas().getGraphicsContext2D();
gc.setLineCap(StrokeLineCap.ROUND);
gc.setMiterLimit(1);

If you need something more localized you could extend the renderer

var renderer = new ErrorDataSetRenderer() {
    @Override
    protected void render(final GraphicsContext gc, final DataSet dataSet, final DataSetNode style) {
        gc.save();
        gc.setLineCap(StrokeLineCap.ROUND);
        gc.setMiterLimit(1);
        super.render(gc, dataSet, style);
        gc.restore();
    }
};

@protogenes
Copy link
Contributor

protogenes commented Sep 28, 2023

@ennerf thanks, I also used a custom renderer.

public class Demo extends Application {
	@Override
	public void start(Stage primaryStage) throws Exception {
		XYChart chart = new XYChart();
		DefaultDataSet dataSet = new DefaultDataSet("miter demo",
				new double[]{1, 2, 3},
				new double[]{1, 2, 1},
				3,
				false);
		dataSet.setStyle("-fx-line-width: 6"); // make it more obvious

		BasicDataSetRenderer renderer = new BasicDataSetRenderer(dataSet);
		chart.getRenderers().setAll(renderer);

		chart.getXAxis().setAutoRanging(false);
		chart.getXAxis().setMin(0);
		chart.getXAxis().setMax(100);

		chart.getYAxis().setAutoRanging(false);
		chart.getYAxis().setMin(0);
		chart.getYAxis().setMax(10);

		primaryStage.setScene(new Scene(chart));
		primaryStage.show();
	}
}

Produces the following render that makes it hard to read the maximum value.
Depending on the overall appearance of the data it can give the wrong impression.
For reference: the default miter limit is 10 which means the line can extend ten times the line width above the value.
grafik

Our real issue looks like this:
grafik

Another issue I encountered: StyleBuilder uses 0x%02x%02x%02x%02x as a color format, but Java-FX and CSS use a # prefix.

ennerf added a commit that referenced this issue Sep 28, 2023
@ennerf
Copy link
Collaborator

ennerf commented Sep 28, 2023

Thanks, the hex string was an oversight.

What would you want the default values to be?

@wirew0rm
Copy link
Member Author

Thanks for bringing this up @protogenes (would even have warranted a dedicated issue i guess.)

From first impulse, a value of 1 seems to be reasonable, since that would from my understanding mean that the value would never exceed the linewidth, which would be what i would expect.
Line Join is a bit more difficult, round seeems plausible but might be a bit more expensive? we usually don't have line widths where that would make a real difference.

Imo it would be sufficient to change it to a reasonable value if not someone else thinks of a usecase where it would have to be user modified. If you would prepare a PR to change it in the renderer and add some before after screenshots i would gladly merge that.

wirew0rm pushed a commit that referenced this issue Sep 28, 2023
@protogenes
Copy link
Contributor

protogenes commented Sep 28, 2023

I'm going with

gc.setLineJoin(StrokeLineJoin.BEVEL);
gc.setLineCap(StrokeLineCap.BUTT);

because when exceeding the miter limit the join will fall back to a bevel anyways and these differences/changes in representation might be irritating for our time series. (Think variance in data reduction causing pixels to flicker for live data)

@wirew0rm
Copy link
Member Author

closing this since 11.3.0 is released now. It might take a few hours for it to be available on maven central search, but it's already available in the repository.

@yezhengli-Mr9
Copy link

yezhengli-Mr9 commented Nov 26, 2023

  • change package names: de.gsi -> io.fair_acc.chartfx (change new github coordinate, pom-xml etc.)

Any way to check earlier version of de.gsi for non-internal developer like me? To fix issues like #639.
Not for sure following works

git clone -b 11.2.7 git@github.com:fair-acc/chart-fx.git
git switch -c 11.2.7

Due to renaming, feel not easy to rollback or debug, for example, for #639 -- at least in terms of repo dependencies like BreadCrumbBarSkin.BreadCrumbButton on

import impl.org.controlsfx.skin.BreadCrumbBarSkin;

protected static class CustomBreadCrumbButton extends BreadCrumbBarSkin.BreadCrumbButton {

Feel can debug earlier version then by

  • pom.xml
      <dependency>
            <groupId>de.gsi.math</groupId>
            <artifactId>chartfx-math</artifactId>
            <version>${chartfx.project.version}</version>
        </dependency>
        <!-- https://mvnrepository.com/artifact/de.gsi/microservice -->
        <dependency>
            <groupId>de.gsi</groupId>
            <artifactId>microservice</artifactId>
            <version>${chartfx.project.version}</version>
        </dependency>
<!-- https://mvnrepository.com/artifact/org.controlsfx/controlsfx -->
        <dependency>
            <groupId>org.controlsfx</groupId>
            <artifactId>controlsfx</artifactId>
<!--            <version>11.2.0</version>-->
            <version>11.0.1</version>
        </dependency>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants