RFR: 8332895: Support interpolation for backgrounds and borders [v18]

[John Hendrikx]

On Fri, 9 Aug 2024 22:36:03 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Sorry to keep harping on this, but so far, I think all the options are quite poor from the perspective of the user, and from a perspective of "could it have been designed this way in the first place". 
>> 
>> I also think I smell something fishy.  The `StyleConverter` seems to serve two unrelated purposes. One is to convert raw parsed values for the CSS parser (`convert(ParsedValue, Font)`) to a Java object.
>> 
>> The other is to support sub-properties (which you can also detect by checking if `CssMetaData.getSubProperties` is not empty).  Although that method is also called `convert` it is only used by  `CssStyleHelper`, which is completely unrelated to parsing.  A more accurate name for that method would be "consolidateSubPropertyValues" (and the opposite would then be something like "extractSubPropertyValues", or `consolidate` and `extract` for short).
>> 
>> There does not seem to be any overlap between these two purposes.  That is it to say, a style converter that is used for  decoding sub-properties is never used for parsing, and vice versa.  FX also does not support special syntax for specifying say a border in short hand form (like `-fx-border: dashed red`).
>> 
>> I think it may be a good idea to perhaps have a look if these purposes shouldn't just be split.  On the one hand, you have a style converter which uses `convert` for parsing.  On the other hand, you have `CssMetaData`, which when it has  sub-properties **must** support a back-and-forth translation (`convert` + `convertBack`) -- this is currently not enforced (ie. you can create `CssMetaData` with sub-properties but then **not** implement `convert(Map)` in the used style converter... this causes problems later on.
>> 
>> The only CSS property currently that supports both short-hand and sub-properties is actually the newly introduced `transition` property.  This could either implement two interfaces, or it could just be two classes.
>> 
>> I've done a quick attempt to do such a split (not looking too much at API) and the whole thing compiles and passes all tests still.  This is surprising, because I deleted several pieces of code, that are apparently never used (or perhaps not tested for, we can investigate this further).
>> 
>> See here: #1533
>> 
>> Diff: https://github.com/openjdk/jfx/pull/1533/commits/8f6d1e56a43068184599ad1ea47b4da98eb70bff#diff-fc96cf3204909a6110e64c37a935927be56efedc9f3f4f6f6843cc4fa7da5a1f
>> 
>> Let me know if you feel this has merit.
>
> I also think that we're probably dealing with two different things that are mixed up in `StyleConverter`. But instead of having this discussion here, it probably makes more sense to have it in the context of [JDK-8333121](https://bugs.openjdk.org/browse/JDK-8333121) because it seems relevant to eventually support both short-hand and long-hand property notations.
> 
> I've renamed `WithReconstructionSupport` to `SubPropertyConverter` and moved it to an internal package for now. Since both `BackgroundConverter` and `BorderConverter` are not public, we can change it any way we want at a later time.

Yes, that's a good idea, I was mainly worried about introducing a new public API that we can't remove later when we haven't figured out yet what the best direction would be.  With this API removed, I think we can get this merged.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1712627157