HanSolo / medusa

A JavaFX library for Gauges
Apache License 2.0
688 stars 129 forks source link

Performance #143

Open anotherprogramer opened 6 years ago

anotherprogramer commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

anotherprogramer commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

This method is a good candidate for interpolation search.

anotherprogramer commented 6 years ago

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 commented 6 years ago

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.