On Mon, 24 Apr 2023 06:01:58 GMT, John Hendrikx <jhendr...@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. > I'm still not sold on the `override` method. Would it not be better to make > the map writable (with `put`) and if there is need to reset properties back > to default offer a `reset` method? I'm saying this because adding an override > method makes the map effectively writable (although delayed, if that's not > going to be changed), and in that case I'd prefer using existing map methods > for this purpose. Overriding a value is not the same thing as just changing it, since it also prevents further automatic updates of the value if the underlying system preference is changed. Since the semantics of overrides are quite different, I wonder if it’s a good idea to squeeze this into the `put` box, instead of just choosing a box with a different name. There are additional problems: if the map is fully mutable, it stands to reason that mappings should also be removable. But what if a mapping was removed, and the underlying system preference is changed? Do we add the mapping again (probably not)? In the end, we have a map where special rules apply for some of its mappings, even mappings that are not even in the map. All that just to avoid the `override` name? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1519456140 PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1521710571
