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

Performance #143

Open
anotherprogramer opened this issue Apr 21, 2018 · 8 comments
Open

Performance #143

anotherprogramer opened this issue Apr 21, 2018 · 8 comments

Comments

@anotherprogramer
Copy link

anotherprogramer commented Apr 21, 2018

I really do like the design of the gauges!
But i was not impressed by the performance.
Then i found this in the Helper Class:

public static final void adjustTextSize(final Text TEXT, final double MAX_WIDTH, double fontSize) {
final String FONT_NAME = TEXT.getFont().getName();
while (TEXT.getLayoutBounds().getWidth() > MAX_WIDTH && fontSize > 0) {
fontSize -= 0.005;
TEXT.setFont(new Font(FONT_NAME, fontSize));
}
}

I did some testing and yes it is as bad as it looks! I know correct textsize is not easy but deleting it speeds up prozess time of a gauge significant. The modern Skin was fireing it at every change.

@HanSolo
Copy link
Owner

HanSolo commented Apr 21, 2018

Thank's for the heads up, what about providing an example of the bad performance so I can work on that. You might also want to provide a better solution if you have one but keep in mind that it should work for all skins :)

@HanSolo
Copy link
Owner

HanSolo commented Apr 21, 2018

I've made some quick modifications and I really can't see the performance issue, so please check with the latest version.

@anotherprogramer
Copy link
Author

anotherprogramer commented Apr 21, 2018

The code above needs between 2 and 10 millisec to complete on my i5. After removing it the needleRotation is nearly instand.

I tryed to find a better solution and failed.
The Solution that i use right is just a dirty hack that makes sure the scale kindof fits.

Btw i just found out 30 sec ago that the linar gauge cant handle Double.NaN.

Also i removed in FGauge :


 if (null != skin && gauge.getSkin().getClass() != GaugeSkin.class) {
            throw new RuntimeException("Please change Skin to GaugeSkin.");
        }

Now i can throw all kind of Nodes into the FGauge and the Class does not seem to mind and creates me Backgrounds for whatever i like.

EDIT: Perhaps other changes in the skins that i did triggered the method more often then it was intended to. I will look into it again.

@anotherprogramer
Copy link
Author

anotherprogramer commented Apr 21, 2018

Ok
I did some excessiv testing.
The Problem exists but is not as big as i thought.

I must had changes earlyer in the file that triggered the above function way to often and not only when the size of the Valuetext changes a decimal. So the really bad performance was mainly due to my alterations of the code .

However i wrote a TestSensor and observed the original version vs my hacked version on 50 gauges triggered every sec, animated. You can see the lag of the orignial version when the value changes a decimal!

@HanSolo
Copy link
Owner

HanSolo commented Apr 21, 2018

Yep I know that this method is time consuming, the question really is if you need to animate 50 gauges at the same time and if si the next question is if they need to be animated because if it is realtime data you better switch animated off. But that’s up to you :)

@protogenes
Copy link
Contributor

This method is a good candidate for interpolation search.

@anotherprogramer
Copy link
Author

anotherprogramer commented May 16, 2018

Lol. you are right. Kind of embarassing that i did not suggest that myself.
I did think about it way to complicated. It is kind of strange that there is no generell accepted awnser to this problem. You would think that its quite common in javafx.

My own idea was to calculate it for the "maximum value" with all "decimals" on creation/redraw and then never change the font again.
In the end i think its even better if the font is static for a gauge.
This way the user has an indirect natural understanding of the range when he looks at the value only.

@anotherprogramer
Copy link
Author

just thought about it again. Because min value can be negative and so longer then max, it would be best to calculate it for MAX and MIN and take the smaller Font of the too.

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

No branches or pull requests

3 participants