On Mon, 24 Apr 2023 19:24:31 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> 1. ObservableMap. Is it unmodifiable? I'd expect it to be, since this API
> exposes the properties of the underlying platform.
Yes and no. It is unmodifiable as it relates to the `Map` contract: you can't
add, remove, or change mappings. However, you can _override_ mappings. For
example, invoking `Platform.getPreferences().override("Windows.UIColor.Accent",
Color.RED)` will change the _effective value_ of the mapping. This is also true
for platform-independent properties:
`Platform.getPreferences().setAppearance(Appearance.DARK)` will override the
platform appearance with the specified value.
Overriding a property or mapping with a new value causes the new value to have
a higher precedence than the platform-provided value. If the override is
removed by calling `override("Windows.UIColor.Accent", null)`, or
`setAppearance(null)`, the platform-provided value is restored, i.e. the
effective value now resolves to the immutable platform-provided value. This
elaborate system ensures that platform-provided values are immutable and you
can't "lose" them by overwriting or removing the values.
> 2. Does this API track user changes (via OS widgets such as Control Panel in
> Windows) at runtime?
Yes, it tracks changes in real-time when they occur. It even tracks changes for
overridden preferences, and resolves the _current_ platform-provided value once
the override is removed.
> 3. "Platform Preferences" name. We already have java.util.prefs.Preferences,
> the name might be confusing. Swing has UIDefaults, perhaps we could think of
> something along those lines?
I prefer "Preferences" for two reasons: it doesn't close the door to adding
other kinds of platform-provided preferences in the future, which might not
directly relate to graphics. Additionally, "UIDefaults" or something along
those lines implies, well, that it's the default. But it's not really that;
this API represents the _current_ configuration, not the _default_
configuration.
> 4. Re: Property API. I think it is still a valid approach, once a call to
> list all the available keys is added. It would be nice to have a way to also
> obtain the type of the value for a specific key, or infer the type from the
> type of property returned. Even more, it might be nice to be able to query
> the:
>
> * list of all the keys exposed by the API
> * value type
Do you mean that you want `Preferences.appearanceProperty()`,
`Preferences.backgroundColorProperty()` and
`Preferences.foregroundColorProperty()` to be returned as a list? For example,
by adding another method like:
interface Preferences {
...
List<ObjectProperty<?>> getProperties() {...}
...
}
I don't really see the added value.
> * possibly, a human-readable description of the property
Why? If it is human-readable, do you envision this to be shown in the user
interface of an application? If so, we'd also need it to be localizable.
However, I don't see why the API should provide this information. Seems like it
could be included in the documentation, though.
> * possibly, whether the value can be changed by this application
All preferences can be overridden by an application, even computed properties
like `Preferences.appearanceProperty()`.
> I am still not sold on the concept of Appearance (what if a Dusk mode will be
> added in addition to the Dark and Light?). However, it could be just another
> platform-specific key, could it not?
At this point, I think that light and dark mode are deeply baked into the
operating systems (at least when it comes to macOS and Windows). Please note
that this aligns with a follow-up enhancement that would add a
`Stage.preferences` property to control the platform appearance of the window
decorations (see [Stage
appearance](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548#stage-appearance)).
Supporting different OS appearances in client applications is quite hard, so I
don't expect operating systems to add more than those two appearances in the
forseeable future. But if that happens, we'll have two options:
1. If _Dusk Mode_ is widely supported by different operating systems, we can
add it to the platform-independent `Appearance` enumeration. However, this
would be a breaking change and not something we would do lightly.
2. If it is just a narrow feature of a single operating system, we can add a
platform-specific key to the `Preferences` map, and style themes that want to
support this particular appearance will then include code to handle just this
platform-specific key.
> [discussion] the problem with ObservableMap is that it drags in a large API
> surface of Map.
I outlined several alternatives to using `ObservableMap`
[here](https://github.com/openjdk/jfx/pull/1014#issuecomment-1500535347), and
concluded that `ObservableMap` is my preferred solution. The main reason is
that, by the time you add in all of the operations that we would like to have
(querying available keys, iterating over the preferences, observing changes),
you'll end up with what is functionally an `ObservableMap`. If we end up
reinventing a map, I think we should actually make it a map.
I don't see much wrong with introducing the `Map` API surface. After all, this
_is_ a map of preference keys to their respective values. I'll go out on a limb
and say that _not_ making a map a `Map` is a worse API choice.
> [discussion] A native window appearance might be more than one attribute. In
> that sense, I think it is better to treat them as such - as platform-specific
> attributes.
>
> (An example I have in mind is a gradient color accent in Windows 10 - I don't
> know if that's still a thing, or how it's done in windows 11 and the dark
> mode). But the idea of limiting the API to a very narrow set of existing
> variants bothers me.
Accent color and dark mode are two separate things, and they have different
representations in the proposed API. The proposed `Stage.appearance` property
controls exactly one bit of information:
On Windows, it sets the `DWMWA_USE_IMMERSIVE_DARK_MODE` flag.
On macOS, it changes the appearance from `NSAppearanceNameAqua` to
`NSAppearanceNameDarkAqua`.
At this point in time, I can't imagine that this will change in the forseeable
future. If at some point we'll get a third mode, then we can evaluate whether
we want to add a third value to the `Appearance` enumeration.
However, you are making a point that is certainly true: the window appearance
is actually _more_ than a single attribute. For example, we currently control
other aspects of native window borders with `StageStyle`. Additionally, there
are several aspects of a native window border that we currently can't control.
For example, in Windows 11, you can choose whether a window has regular rounded
corners, small rounded corners, or no rounded corners. This is controlled by
`DWM_WINDOW_CORNER_PREFERENCE`, but a JavaFX application has no way of
controlling this preference.
My point is this: I agree that there are additional knobs that control how
native windows are rendered. What I've been calling `Appearance` in this PR is
_one_ of those knobs. I don't want to suggest that this is the only knob, but
it's a knob that controls a well-defined single bit of information.
> [question] when an application changes the value of one or more properties,
> does it change the appearance of that application alone, all of the javafx
> applications, or all of the platform applications?
The `Preferences` map can only read OS prefs, it cannot change them. So when
you override the value of one of the preferences, the change is only visible to
the application alone, and not to other JavaFX applications or the OS.
> I don't quite like the 'override' method and the idea of app modifying the
> values, as I see it as a purely informational API. The changes should, in my
> opinion, be triggered by the platform only. Any other functionality is
> app-specific.
But what do you do when a style theme uses platform preferences to adjust its
appearance (light/dark mode), but an application either doesn't want to offer a
dark mode, or wants the _application_ (not the OS) be able to toggle light/dark
appearance? If an application can't override the preferences that go into a
style theme, then it can't control its appearance.
>
> Another thing i am not comfortable with is the eager initialization (and
> that's another reason I don't quite like ObservableMap idea). Generally
> speaking, we should not be initializing things an application is not going to
> use. That's why I like the concept of accessing individual attributes - if a
> theme needs Appearance, only that value and its machinery gets initialized.
>
> Small delays added here and there add up.
I think you're chasing nanoseconds here. We're talking about a map that
contains a few dozens of entries at max, and that exists as a singleton for the
lifetime of the application. On top of that, from an implementation standpoint,
we're querying all OS preferences anyways on the native side, there is no
fine-grained and elaborate system to only query the preferences that have
actually changed (the compexity just isn't worth the effort).
This is not a performance-critical piece of code, since OS preferences don't
change often. In fact, for most users, they don't change at all while the
applications is running.
> [discussion]
>
> > `Appearance` is central to _all_ of those features
>
> From the API perspective, Appearance, though important, is just one of many
> attributes. Adding a separate method where we could operate with a Set of
> attributes ("hints") is, in my opinion, a less optimal solution.
>
> For example, Stage.setAttributes(Set<?>) is a much more flexible approach. If
> things change tomorrow with new attribute added, or some other change, this
> API does not have to be modified. (The Stage should silently ignore any
> attributes it does not recognize or that are not supported by the platform).
>
> For the Platform Preferences API, a native attribute such as
> `DWMWA_USE_IMMERSIVE_DARK_MODE` may also be echoed by an Appearance
> attribute, so the app dev can use either one depending on the requirements.
>
> I suppose we could have a dedicated getAppearance() method as shortcut (as
> well as some other frequently used shortcuts).
>
> What do you think?
I think you're conflating platform preferences with stage styling here.
`DWMWA_USE_IMMERSIVE_DARK_MODE` is not a platform preference, it is a window
attribute (an implementation detail) that can be set independently from any
preference. `Appearance`, on the other hand, is a platform preference that can
be toggled by users and is directly meaningful for style theme authors and
application developers.
We should focus the discussion here on the preferences API, not on stage
styling. I understand that those two topics have a bit of common ground, but
extended discussions of new `Stage` APIs seem a bit premature.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1520768361
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1520918231
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1520941610
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1520971384
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1531396822
PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1532013105
