I've made what I've believe to be a cleaner fix here: https://github.com/DeanWookey/openjdk-jfx/commit/295a3e136c59eb661021de7b4c7ead8bd36a749a
I'm not sure exactly how to proceed from here. I submitted a bug report several weeks ago so that I could submit a pull request against it, but I've heard nothing back. Should I create a bug report myself since I have author status now? Should I just create a pull request on github to discuss the implementation there, and link the bug report later? Dean On Fri, Jan 18, 2019 at 10:28 AM Dean Wookey <wookey.d...@gmail.com> wrote: > I've only tested with our application that includes all those things. We > set a font size on the root node and everything is done in em. The user can > change the font size of the root node, which filters down to things like > tooltips, context menus, tables etc. > > Do you know if there are tests specifically for these things? > > Otherwise, I'm not 100% happy with this fix because I feel it makes things > more confusing. In my opinion the lookupFont should cache itself, or there > should be a cachedLookupFont which does this. I don't really understand the > special cases of SKIP and null however, so I replicated the steps used to > cache the result elsewhere. I think it would be better if someone else made > a more comprehensive fix which made things a little clearer. > > Dean > > On Thu, Jan 17, 2019 at 6:41 PM David Grieve <david.gri...@oracle.com> > wrote: > >> I think the change looks reasonable. Has it been tested against things >> like changing -fx-font in some parent, or setting the font property in some >> parent? What about popups? If I have a tooltip on a Label, for example, >> does the tooltip properly pick up font from the Label's style or font >> property? >> On 1/17/19 11:10 AM, Dean Wookey wrote: >> >> This is almost a year old, but I think I've found a partial solution >> (David's suggestions on a top down approach still make sense, but that's a >> bigger issue). Caching an uncached call to lookupFont definitely improves >> performance. >> >> >> https://github.com/DeanWookey/openjdk-jfx/commit/e12f00fb6c2fc690c0a7fb089f7b0c81d6930fa9 >> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_DeanWookey_openjdk-2Djfx_commit_e12f00fb6c2fc690c0a7fb089f7b0c81d6930fa9&d=DwMFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=H6jKHjHoLf6vCNF0PYEnJ6cS6kScbGLraJqgoRs9znU&m=xH8dfYiQe6JO5VXucoVgtnsubTZNAhr0XVK5OaSDDbs&s=YvsZxESGrm40L34aR16E2fQCKjh8tCQ311R3h63hu0c&e=> >> >> Using a stack of 50 stackpanes instead of 20, I now get the following: >> >> Before modification: >> >> With setting font size: 10675ms >> Without setting font size: 49243ms >> >> After modification: >> >> With setting font size: 10852ms >> Without setting font size: 20357ms >> >> Dean >> >> On Thu, Apr 19, 2018 at 9:51 PM David Grieve <david.gri...@oracle.com> >> wrote: >> >>> I was thinking about https://bugs.openjdk.java.net/browse/JDK-8177635 >>> and https://bugs.openjdk.java.net/browse/JDK-8090462 >>> >>> There is also https://bugs.openjdk.java.net/browse/JDK-8187955 >>> >>> On 4/19/18 3:02 PM, Nir Lisker wrote: >>> >>> I think this is the issue: >>> https://bugs.openjdk.java.net/browse/JDK-8088615. There's also >>> https://bugs.openjdk.java.net/browse/JDK-8193445. >>> >>> - Nir >>> >>> On Thu, Apr 19, 2018 at 1:42 PM, David Grieve <david.gri...@oracle.com> >>> wrote: >>> >>>> Resolving the relative size involves a lot of lookup. You have to go up >>>> the scene-graph from the child to find a font style. If you get to the root >>>> and haven't found a font style, then use the default font. Performance in >>>> this area could be vastly improved by passing the size from either a font >>>> style or the default font down the scene-graph as styles are evaluated. >>>> There are other style lookups that could benefit from this as well, >>>> resolving looked-up colors for example. I believe I created a bug for this >>>> a long time ago. >>>> >>>> >>>> >>>> On 4/19/18 5:57 AM, Dean Wookey wrote: >>>> >>>>> Hi All, >>>>> >>>>> In our application we add and remove a lot of nodes to the scene graph >>>>> regularly, and also make use of em font sizes to scale certain parts >>>>> of our >>>>> application. We've noticed performance issues when adding nodes to the >>>>> scene, and it seems to be related to em sizes in our css. >>>>> >>>>> As a test we added a chain of 20 stackpanes to a root stackpane. On the >>>>> root, we set a font size of 8pt via inline css. We then added and >>>>> removed a >>>>> new tableview from the deepest stackpane 500 times, waiting for the >>>>> node to >>>>> render after each add and remove. >>>>> >>>>> In each of the experiments, we compared adding tableviews without css >>>>> and >>>>> adding tableviews with an inline css font size of 8pt. We then tried >>>>> setting different font sizes in the stackpane chain. >>>>> >>>>> I've attached sample code for these experiments. >>>>> >>>>> The results (on jdk 9.0.4 - jdk 10 was similar) are as follows: >>>>> >>>>> Setting a 1em font size on all stackpanes except the root. >>>>> With font on tableview: 14707ms >>>>> Without font on tableview: 27725ms >>>>> >>>>> Setting a 1em font size on the first child of the root only. >>>>> With font on tableview: 14221ms >>>>> Without font on tableview: 19187ms >>>>> >>>>> Using the original setup with no additional fonts. >>>>> With font on tableview: 13990ms >>>>> Without font on tableview: 13847ms >>>>> >>>>> It looks like using a relative font size has a large effect performance >>>>> wise on descendant nodes. I would expect some amount of font size >>>>> caching >>>>> in the chain of stackpanes since I'm reusing the same chain and >>>>> adding/removing nodes from that chain repeatedly. >>>>> >>>>> I'm not sure how valid my test is, or how much of an issue this really >>>>> is >>>>> in practice? >>>>> >>>>> Dean >>>>> >>>> >>>> >>> >>>
