On Mon, 24 Apr 2023 06:23:41 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

> > I'm not convinced that a delayed change + commit system is the correct way 
> > to do this. Properties should behave the same everywhere in JavaFX and this 
> > seems to change how they work quite a bit.
> > Instead, I propose to look at how layout in JavaFX is handling this 
> > problem. Layout looks at thousands of properties, yet changing one or many 
> > of the involved properties does not involve an expensive layout 
> > recalculation per change. Instead, changes are tracked by marking certain 
> > aspects of the involved controls dirty. On the next pulse, the layout code 
> > notices that something that would influence layout and CSS decisions has 
> > changed, and performs the required changes. The properties involved are all 
> > normal properties, that can be changed quickly, reflect their current value 
> > immediately and that can be overridden by the user or reset back to 
> > defaults. There is no override or commit system needed.
> > Have you considered allowing users to change preference values directly, 
> > but not acting on those changes until the next pulse occurs? Users can 
> > still listen for keys/properties, just like they can for layout properties, 
> > but the major changes that involve recomputing CSS is only done once per 
> > pulse.
> > This would make it possible to change several preference values without 
> > penalty (which happens on the FX thread anyway, so pulses are on hold 
> > during that time), and they're automatically "committed" once the user is 
> > done on the FX thread and the next pulse fires. I think it would be a very 
> > good fit.
> 
> I think this could work, but it also means giving up on instant change 
> notifications. A call to `setAppearance` or `override(key, value)` will not 
> instantly fire a change notification (after the code that modifies the 
> properties is done), but delay it until the next pulse. Resetting the value 
> to its original value before the next pulse would probably also elide the 
> change notification entirely. It's basically the same as change+commit, but 
> with an implicit commit in the next pulse.

That's not quite what I meant. You can add listeners still and get instant 
change notifications.  Just like when I listen to the `backgroundProperty` of a 
control, and someone changes it, I get notified instantly and the change is 
reflected (in the property) instantly.  Only when the next pulse fires is the 
change actually rendered.

I would think the same is possible with say the appearance property.  When I 
change it from LIGHT to DARK, everyone interested gets this notification 
immediately.  On the next pulse, the change is noticed and only then do we 
change the stylesheets or make other adjustments that are high impact.  
Basically, the computationally expensive stuff happens during a pulse; it could 
register invalidation listeners on properties of interest which just set a 
flag, that is checked and reset on the next pulse.

I'm not 100% sure, but it seems you want to add listeners to these properties 
yourself to instantly trigger the Theme updating code -- I'm saying, only set a 
boolean that they've changed, check it on the next pulse using 
`Scene#addPreLayoutPulseListener` (or perhaps there is another mechanism you 
can tap into internally), and then trigger the Theme change.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1519514059

Reply via email to