On Sat, 3 Aug 2024 00:14:28 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> I see two uses of this interface, both in combination with `Interpolatable`. >> Should this just extend `Interpolatable`, or are there cases to be expected >> where this marker is present but `Interpolatable` is not? >> >> Also, have you considered adding a default method to `Interpolatable` to >> indicate it supports component wise interpolation? >> >> It looks modelled after `RandomAccess`, but the reasons for that being a >> marker interface are not similar to this situation (it is a marker interface >> so it can also mark the old collections, like `Vector`). > > `ComponentTransitionable` is kind of orthogonal to `Interpolatable`. It tells > us to first decompose the object, and then transition each component > separately. `Border` and `Background` implement both interfaces, but > `Interpolatable` is not used for CSS transitions (i.e. `Border.interpolate()` > is not called). > > It doesn't make much sense to add a default method to `Interpolatable` to > indicate component-wise transitions, because we can't use the `interpolate()` > method for component-wise transitions. > > This should be thought of as two paths: we either use `Interpolatable`, or we > use the decompose-reconstruct route (which is only available to CSS, but not > programmatically). > > Now, the name might also be `ComponentInterpolatable`, but this suggests a > close proximity to `Interpolatable` (being in the `javafx.animation` package, > having a programmatic API, etc). However, `ComponentTransitionable` only > works for objects that expose their components to CSS (which is why it is > located in `javafx.css` and not `javafx.animation`). I've taken a closer look, and I have the feeling that `ComponentTransitionable` is more of an indicator that the corresponding `StyleConverter` has implemented `convertBack`. I think noting this information closer to the source would therefore be a good idea. >From what I can see, `applyComponentTransition` already falls back to calling >`set(T)` if there is anything out of the ordinary (like missing CSS meta data >or a missing StyleConverter). For both interpolation cases (`Interpotable` and `ComponentTransitionable`) you are getting the CSS meta data. You can externalize that part. When you have the meta data, you can get the `StyleConverter` and check if it supports deconstruction (instead of having a marker on the final result). You can either: - put the marker on `StyleConverter` - have `convertBack` return an `Optional` or `null` if unsupported (there are already several `null` checks for missing CSS meta data or the missing converter, one more here seems not unreasonable). - have a method on `StyleConverter` that indicates its capabilities (true/false, `SUPPORTS_DECONSTRUCTION`, etc..) The advantage of this IMHO is that it keeps CSS concerns (component transitionable) in the CSS realm (with StyleConverter), while leaving simple classes that are just value holders (Border, Background) untouched. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1702797888
