On Tue, 21 Jul 2026 23:46:00 GMT, John Hendrikx <[email protected]> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line >> 946: >> >>> 944: */ >>> 945: >>> 946: if (!cssMetaData.isSettable(node)) continue; >> >> the contract for `CssMetaData.isSettable()` says "_This method is called >> before any styles are looked up for the given property._" but here it's >> called after the lookup() in L929 > > That's an odd amount of detail that is rather implementation specific. > > I would suggest we remove it, as I don't think there are good reasons for > this other than "that's how it works currently", and it actually is an active > source of problems. > > A property being unsettable does not mean you can't evaluate the stylesheet > to find what value it would have gotten (and that's what we do now). The only > reason this method exists is that is faster than catching "RuntimeException: > A bound value cannot be set.". > > There are two good reasons to still evaluate the CSS value (even if we won't > set it): > > - The property may not be bound forever; if the cache entry was created > without the calculation of the value for a "locked" property, then we later > won't know what CSS based value to put in there (note: unbinding a property > is not detected, so binding/unbinding already plays very badly with the CSS > system). > > - The cache entry may be shared with siblings in the same state; if the cache > entry was initialized by a Node with bound properties, and we then just skip > those, then the other siblings would not have that value either -- this > causes incorrect value resets and properties to not be styled, which this PR > specifically wants to address > > If there are good reasons to keep it, then the only other option we have is > to NOT create a cache entry at all when a Node has any CSS property bound (a > sibling that is in a better state may create it still though). This could be > a performance hazard though, because if it is the only node of its kind, it > would always need to go through the slow `lookup` path until there are no > more bound properties. +1 for removing it. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2218#discussion_r3637218388
