On Tue, 28 Nov 2023 23:34:30 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Provide a public utility method for use by custom and core Nodes to simplify
>> boilerplate around styleable properties as well as to save some memory.
>>
>>
>> /**
>> * Utility method which combines {@code CssMetaData} items in one
>> immutable list.
>> * <p>
>> * The intended usage is to combine the parent and the child CSS meta
>> data for
>> * the purposes of {@code getClassCssMetaData()} method, see for example
>> {@link Node#getClassCssMetaData()}.
>> * <p>
>> * Example:
>> * <pre>
>> * private static final List<CssMetaData<? extends Styleable, ?>>
>> STYLEABLES = CssMetaData.combine(
>> * <Parent>.getClassCssMetaData(),
>> * STYLEABLE1,
>> * STYLEABLE2
>> * );
>> * </pre>
>> * This method returns an instance of {@link java.util.RandomAccess}
>> interface.
>> *
>> * @param list the css metadata items, usually from the parent, must not
>> be null
>> * @param items the additional items
>> * @return the immutable list containing all of the items
>> *
>> * @since 22
>> */
>> public static List<CssMetaData<? extends Styleable, ?>> combine(
>> List<CssMetaData<? extends Styleable, ?>> list,
>> CssMetaData<? extends Styleable, ?>... items)
>>
>>
>> Problem(s):
>>
>> - the same less-than-optimal implementation is duplicated throughout the
>> javafx code base which combines the parent and child styleable properties,
>> using ArrayList wrapped in a Collections.unmodifiableList()
>> - the capacity of underlying ArrayList might exceed the number of items,
>> wasting memory
>> - a potential exists for the custom component developer to inadvertently
>> create a sub-standard implementation (i.e. return a List which does not
>> implement RandomAccess interface), or forget to include parent's styleables.
>>
>> Non-Goals:
>>
>> - not redesigning the lazy initialization of STYLEABLES list
>> - not changing the way styleables are enumerated in Nodes and Controls
>> - not adding any new methods to JDK (collections)
>>
>> To Be Discussed:
>>
>> - it is not clear whether the order of styleables returned by <N extends
>> Node>.getClassCssMetaData() is of any importance
>> - the current spec for Node.getCssMetaData() specifies the return value as
>> "The CssMetaData associated with this node, which may include the
>> CssMetaData of its superclasses.", implying that it may or may not include.
>> It is unclear whether it must include the parent's styleables (well, except
>> the Node whose superclass is Obje...
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
The downside of John's proposal is that the metadata list would now be
duplicated in every node, while currently these lists are only duplicated in
`Control` nodes. The size of these lists is typically around 20-40 elements.
In any case, expanding on what I wrote earlier, here is how the performance
requirements could be specified for `getClassCssMetaData`, if we go for "option
1":
/**
* Gets an unmodifiable {@code CssMetaData} list for the styleable properties
of this class.
* <p>
* If a derived class declares new styleable properties, it must redeclare this
method to hide the
* base method. From the redeclared method, it must return the concatenation of
the base metadata
* and the metadata of the newly-added styleable properties.
* <p>
* Implementations of this method must satisfy the following performance
requirements to prevent
* severe performance degradation in the CSS system:
* <ol>
* <li>The returned list must support random access.
* <li>The method must not allocate a new list on each invocation.
* </ol>
* It is strongly advised to use {@link CssMetaData#combine} to create the
concatenated list and
* store it in a static field:
* <pre>{@code
* private static final List<CssMetaData<? extends Styleable, ?>>
CSS_METADATA =
* CssMetaData.combine(
* <Parent>.getClassCssMetaData(),
* CUSTOM_CSS_METADATA_1,
* CUSTOM_CSS_METADATA_2,
* CUSTOM_CSS_METADATA_3
* );
*
* public static List<CssMetaData<? extends Styleable, ?>>
getClassCssMetaData() {
* return CSS_METADATA;
* }
* }</pre>
* Note that in this example, {@code <Parent>} should be substituted with the
name of the direct base
* class, even if the direct base class does not have its own {@code
getClassCssMetaData()} method.
* This ensures that the code remains correct when such a method is added to
the direct base class.
*
* @return the {@code CssMetaData} list for the styleable properties of this
class
* @see CssMetaData#combine
* @since JavaFX 8.0
*/
public static List<CssMetaData<? extends Styleable, ?>> getClassCssMetaData()
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1296#issuecomment-1832775656
