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

[John Hendrikx]

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

>> Clear, thanks MIchael.  I would suggest `Reconstructable` as the name, which seems more inline with interface naming (especially `Interpolatable`). Reads nice also:
>> 
>>     if (converter instanceof Reconstructable r) { ... }
>
> But the converter is not reconstructable (in comparison to Interpolatable, which is something the marked types actually are).

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.

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

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