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

[Michael Strauß]

On Fri, 9 Aug 2024 19:42:56 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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.

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.

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

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