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

[Andy Goryachev]

On Wed, 7 Aug 2024 23:55:39 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Although I'm not advocating for a specific change, I think looking at how `T convert(Map)` works when it was newly introduced in Java 9 should be taken into account as well.
>> 
>> There seem to be a couple of general approaches:
>> 
>> 1. Have a marker that can be checked from the outside
>>     - Annotation (checked from the outside)
>>         - Unusual for this kind of check
>>     - Marker interface
>>         - Seen more often, but unusual to not put the new method there
>>     - Subtype 
>>         - Bad idea, you can only inherit once
>> 2. Introduce a new interface with the new method
>>     - Defines the method and serves as the marker at the same time
>> 3. Have a method that can be called that guards a 2nd method call
>>     - Doesn't matter how this is fed (constructor, annotation, override)
>>         -  You see this more often, but it's not a great pattern (2 method calls for the price of one...)
>> 4. Return a special value from a method that may be unsupported
>>     - The most obvious (modern) candidate is to return `Optional` but is a bit unusual to indicate support/non-support
>>     - Throw `UnsupportedOperationException` -- although standard, I think it indicates a programmer mistake that should be avoided (in other words, encountering this exception in production code should result in a code fix)
>>     - Return `null`
>> 
>> Now the very last option (returning `null`) was the way this was handled when `T convert(Map)` was introduced in Java 9.  Even though it may not be the prettiest solution, it has two things going for it:
>> 
>> - It's fast (probably the fastest of the above options, although for most by just a slim margin)
>> - It fits in with the existing design
>> 
>> So, my order of preference:
>> - Just return `null`
>> - A new interface, which includes the new method
>> - A marker only interface
>> - A supportsXYZ method (regardless of how that is approached)
>> 
>> I don't think I could get behind any of the other options, because they either stick out like a sore thumb versus the existing design, limit future options or just perform too poorly.
>
> Returning `null` seems fine from the perspective of `StyleConverter`, but it makes the calling code very awkward. Remember, we ended up here because we needed a way to detect whether an object would support component-wise transitions. If we can't detect that without invoking `convertBack`, we would need to either:
> 1. Speculatively decompose the value without knowing whether there even are any transitions defined on the node. This is bad because most of the time, there will be no transitions; we will end up deconstructing many objects for no reason.
> 2. _Assume_ that an object is component-transitionable, look up all potential transitions, and decompose the value; then, if we were wrong with out assumption, go back to the start and try again with another code path (`Interpolatable` or no transition).
> 
> Instead, what I've implemented now is a new interface `StyleConverter.WithReconstructionSupport`, which contains both methods:
> 
> public interface WithReconstructionSupport<T> {
>     T convert(Map<CssMetaData<? extends Styleable, ?>, Object> values);
>     Map<CssMetaData<? extends Styleable, ?>, Object> convertBack(T value);
> }
> 
> 
> This allows us to use object deconstruction and reconstruction with a single interface reference, and also allows us to detect such support very easily.

+1 for `StyleConverter.WithReconstructionSupport`

maybe use a shorter name, like

`StyleConverter.WithReconstruction`

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

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