palexdev/MaterialFX

New theming API causing performance issues

Closed this issue · 17 comments

MFXCircleToggleNode still showing text even when specifying through fxml and java to show graphics only:
buttonToggleNode_pinLeftPane.setContentDisplay(ContentDisplay.GRAPHIC_ONLY);

image

Affected versions: tested on 11.14.0 and 11.16.1, same behavior

Complementary evidence:
image

In my fxml, it looks like that:

<MFXCircleToggleNode fx:id="buttonToggleNode_pinLeftPane" contentDisplay="GRAPHIC_ONLY" gap="0.0" graphicTextGap="0.0" minHeight="-Infinity" minWidth="-Infinity" prefHeight="35.0" prefWidth="41.0" size="15.0" styleClass="mfx-button" stylesheets="@../../css/nodes/button/ChecksRadiosToggles.css" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> <graphic> <ImageView fx:id="imageView_pinLeftPane" accessibleRole="PARENT" fitHeight="20.0" fitWidth="20.0" pickOnBounds="true" preserveRatio="true"> <image> <Image url="@../../../icons/thumbtack.png" /> </image> </ImageView> </graphic> <padding> <Insets top="-28.0" /> </padding> </MFXCircleToggleNode>

EDIT: Latest stable version: 11.14.0-EA3

Also, starting from one version above (13.15.0), MFXDatePicker UI is all messed

Looking at the Complementary evidence you provided, it looks like your aren't applying MFX Themes. Incase you really haven't added MFX Themes to your application, this can be done by adding the lines below to your Application's start() method:

MFXThemeManager.addOn(scene, Themes.DEFAULT, Themes.LEGACY);
                        io.github.palexdev.mfxcomponents.theming.MaterialThemes.PURPLE_LIGHT.applyOn(scene); // Select the theme color of your preference.

Running the FXML you provided minus the CSS and using a different image resource on my side works as expected.
Zenmart ERP 9_30_2023 9_38_23 AM

@infinite-dev22,

You're absolutely correct; I forgot to include the MFXThemeManager. Thanks to your suggestion, I've now successfully updated to the latest MaterialsFX version without any issues. Much appreciated!

On another note, I didn't add the line:

io.github.palexdev.mfxcomponents.theming.MaterialThemes.PURPLE_LIGHT.applyOn(scene);

I have a somewhat intricate styling system in my application, so I was concerned about how this "PURPLE_LIGHT" theme might interact with my existing styles. Moreover, I'm not entirely sure what this line of code does. Please let me know if I can omit it without any problems.

I haven't tried working without the said line of code. Though if one doesn't like the PURPLE_LIGHT theme, they can opt for the other variables available. The theming basically gives a primary color and may be a secondary color(I haven't observed yet) to the application components. This can still be overriden with one's personal coloring still though for those that don't have any problem with the styling may not further style their components.

For @infinite-dev22 and everyone reading this update:

DO NOT USE MFXThemeManager.addOn(scene, Themes.DEFAULT);

If your application is simple and don't have that much nodes, it's ok. Otherwise, if your application is production ready and has thousands of nodes, don't apply this line of code in your project, otherwise your application performance will highly decrease.

I haven't done any study on that in order to provide data showing the performance decrease, but that shouldn't be any hard.

For now, the greatest contribution I can make is provide this link

"Binary CSS" and "Visits when applying CSS" explains the issue. Long story short, all the css files inside Themes.DEFAULT (dozens of files) searches for every single node in scene graph to apply the style instead of applying the style only to the need it's desired.

Going a little bit further, if you go to youtube and search for "oracle javafx performance tips and tricks" you should find a video explaining the issue as well.

How about the fix? Apply the css file ONLY to the desired node instead of the entire scene. I can explain further if needed

found a fix

Can you share the fix please?

  1. It's a known issue that I also tried explaining to @stefanofornari some time ago in the Discussions section
    Applying the entire theme to a Scene seems a big no-go and also comes with invoveniences such as the fact that dialogs and popups, which are separate scenes, won't be styled
  2. The solution should be to set the theme as the Application user agent stylesheet, but this method is not implemented on the main branch and it's available only on the rewrite branch (if you haven't seen it, I suggest you to check the theming package there, there's a huge work to improve Theming
  3. A MRE would be appreciated. I'll try to test the app @stefanofornari gave me some time ago and see if the performance is back with the new method

@stefanofornari check this Video showcasing the upcoming fix. The first run is with old MaterialFX, the second is with version 11.16.1, the last is with a local version

@FelipeAumannRS if you can provide an MRE project I can test the fix for you too, OR if you guys want I can share here the JAR containing the changes

oh, great stuff @palexdev, it looks like a huge improvements. At the moment I am pretty busy on another project, I would not be able to try the app with the new MFX.

oh, great stuff @palexdev, it looks like a huge improvements. At the moment I am pretty busy on another project, I would not be able to try the app with the new MFX.

No problem, hope to get some feedback from @FelipeAumannRS too. For now, I'll wait, eventually I'm going to push changes and release a new version.

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

No problem, hope to get some feedback from @FelipeAumannRS too. For now, I'll wait, eventually I'm going to push changes and release a new version.

yeah I guess pushing it out does not harm at all.

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

I may fund it! ;)

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

Fair enough!

@FelipeAumannRS if you can provide an MRE project I can test the fix for you too

I really appreciate it, but unfortunately I have no such project

OR if you guys want I can share here the JAR containing the changes

Yes, please! That way I can test it. Just to make sure, in order to understand what is going on with the .jar and aiming to have a better understanding of what the new MFX version is doing, what is the branch you've been working on?

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

Fair enough!

@FelipeAumannRS if you can provide an MRE project I can test the fix for you too

I really appreciate it, but unfortunately I have no such project

OR if you guys want I can share here the JAR containing the changes

Yes, please! That way I can test it. Just to make sure, in order to understand what is going on with the .jar and aiming to have a better understanding of what the new MFX version is doing, what is the branch you've been working on?

No problem, hope to get some feedback from @FelipeAumannRS too. For now, I'll wait, eventually I'm going to push changes and release a new version.

yeah I guess pushing it out does not harm at all.

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

I may fund it! ;)

I'll share the champagne theme hahaha

If this crap is finally fixed once and for all, I'm gonna pop a champagne bottle 🫠

Fair enough!

@FelipeAumannRS if you can provide an MRE project I can test the fix for you too

I really appreciate it, but unfortunately I have no such project

OR if you guys want I can share here the JAR containing the changes

Yes, please! That way I can test it. Just to make sure, in order to understand what is going on with the .jar and aiming to have a better understanding of what the new MFX version is doing, what is the branch you've been working on?

I'm not at home right know, expect a response with the attached jar in a couple of hours.

The jar is from the main branch. The Theming API is essentially a backport of what's on the rewrite branch, with just a few needed adjustments to make it fit.

Detailed Explanation JavaFX doesn't allow to set more than one global user agent stylesheet. And this is very inconvenient for libraries with custom components such as MaterialFX. As I already said before, adding the whole theme on the main scene comes with some pros but at least two huge downsides: only that scene will be styled, parse/process the whole theme on each scene causes a huge performance degradation. Not so long ago, I decided to tackle this issue on the rewrite branch. I developed a system that takes a set of CSS stylesheets and combines them into one single CSSFragment. This way you can combine the default JavaFX theme (MODENA) with third party themes (MaterialFX). The system is not perfect and requires to follow some specific rules when writing CSS, please read the documentation, I'm serious. Also, since it's experimental, I would like to receive feedback whenever an issue occurs, I'd be happy to improve it. Last but not least, there is a PR on the JavaFX repo that allows using more than one global user agent stylesheet, but I believe we won't see this feature for a long time. Once it is merged, this system will be deprecated and removed

@FelipeAumannRS here is the jar: mfx.zip
It's the shadow jar variant, includes all the dependencies

To build and set the user agent stylesheet, add this to the application start method:

UserAgentBuilder.builder()
    .themes(JavaFXThemes.MODENA) // This can be avoided if you are not going to use any JavaFX components, although I still recommend using it anyway
    .themes(MaterialFXStylesheets.forAssemble(false)) // if you are using legacy components then pass 'true' as argument
    .setDeploy(true)
    .setResolveAssets(true)
    .build()
    .setGlobal();

@FelipeAumannRS still no feedback 😅

Should be fixed in version 11.17.0