[External] : Re: Some classes could be sealed
Kevin Rushforth
kevin.rushforth at oracle.com
Wed Feb 1 23:58:54 UTC 2023
> You can't extend these without tampering with internals.
Pretty much, yes.
Node is an abstract class that requires a concrete implementation to be
useful. The set of subclasses that can be used in describing and
rendering the scene graph is a finite and known set. The rendering of
the scene graph is an implementation detail; each node in the scene
graph has a corresponding peer (an NGNode subclass) that is needed to
implement various node types (shapes, images, etc).
So Node, as well as its abstract subclasses, like Shape, Shape3D,
Camera, and LightBase, needs a known concrete subclass in order to do
anything. Similarly, Material (which is not a Node) is abstract and has
implementation that cannot be provided by an application class.
By contrast, Parent can be usefully subclassed. It is a concrete class
that is used as a container for other nodes, and has implementation of
layout, traversal, bounds computation, etc.
--- Kevin
On 2/1/2023 2:48 PM, Nir Lisker wrote:
> For Material and LightBase it's because they are just facades whose
> implementation is in native code. You can't extend these without
> tampering with internals. I think that Camera and Shape3D also
> requires modifying internal stuff, though not at the native level.
>
> On Thu, Feb 2, 2023 at 12:38 AM John Hendrikx
> <john.hendrikx at gmail.com> wrote:
>
> I'm curious to know why these classes are not allowed to be
> subclassed directly, as that may be important in order to decide
> whether these classes should really be sealed.
>
> --John
>
> On 01/02/2023 20:37, Kevin Rushforth wrote:
>> I read the spec for sealed classes more carefully, and it turns
>> out we can't make Node sealed. At least not without API changes
>> to SwingNode and MediaView (and implementation changes to
>> Printable in the javafx.web module). All of the classes listed in
>> the "permits" clause must be in the same module, and SwingNode
>> (javafx.swing) and MediaView (javafx.media) extend Node directly
>> they would need to be "permitted" subtypes, but there is no way
>> to specify that. We would also need to do something about the
>> tests that extend Node and run in the unnamed module. So this
>> doesn't seem feasible.
>>
>> We could still seal Shape, Shape3D, LightBase, and Material,
>> since all permitted implementation are in the javafx.graphics
>> module. It may or may not be worth doing that.
>>
>> -- Kevin
>>
>>
>> On 2/1/2023 9:45 AM, Nir Lisker wrote:
>>> I'll add that internal classes, mostly NG___ peers, can also
>>> benefit from sealing. NGLightBase is an example.
>>>
>>> Material is another public class that can be sealed.
>>>
>>> On Wed, Feb 1, 2023 at 7:37 PM Kevin Rushforth
>>> <kevin.rushforth at oracle.com> wrote:
>>>
>>> I agree that we should only seal existing classes that could
>>> not have been extended by application classes. The ones I
>>> listed in my previous email fit that bill, since an attempt
>>> to subclass them will throw an exception when it is used in
>>> a scene graph. Each documents that subclassing is disallowed.
>>>
>>> Btw, we've already started making use of pattern-matching
>>> instanceof in the implementation anyway. It would be the
>>> first API change that relies on a JDK 17 feature, but for
>>> JavaFX 21, I see no problem in doing that.
>>>
>>> -- Kevin
>>>
>>>
>>> On 2/1/2023 9:06 AM, Philip Race wrote:
>>>> In the JDK we've only sealed existing classes which
>>>> provably could not have been extended by application classes,
>>>> so I'm not sure about this ..
>>>>
>>>> also I think that might be the first change that absolutely
>>>> means FX 21 can only be built with JDK 17 and later ..
>>>>
>>>> -phil
>>>>
>>>> On 2/1/23 8:59 AM, Thiago Milczarek Sayão wrote:
>>>>> Yes, sorry, I made the email title in plural, but I meant
>>>>> what Michael said, Node would be sealed permitting only
>>>>> what is needed for JavaFx internally.
>>>>>
>>>>>
>>>>> -- Thiago
>>>>>
>>>>>
>>>>> Em qua., 1 de fev. de 2023 às 13:48, Michael Strauß
>>>>> <michaelstrau2 at gmail.com> escreveu:
>>>>>
>>>>> I don't think that's what Thiago is proposing. Only
>>>>> `Node` would be sealed.
>>>>> The following subclasses would be non-sealed: Parent,
>>>>> SubScene,
>>>>> Camera, LightBase, Shape, Shape3D, Canvas, ImageView.
>>>>> And then there are additional subclasses, which don't
>>>>> fit into this
>>>>> idea since they are in other modules: SwingNode (in
>>>>> javafx.swing),
>>>>> MediaView (in javafx.media), Printable (in javafx.web).
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Feb 1, 2023 at 5:39 PM John Hendrikx
>>>>> <john.hendrikx at gmail.com> wrote:
>>>>> >
>>>>> > I think this may be a bit unclear from this post,
>>>>> but you're proposing I think to make `Node`, `Shape`
>>>>> and `Shape3D` sealed. For those unaware, you're not
>>>>> allowed to extend these classes (despite being
>>>>> public). For example Node says in its documentation:
>>>>> >
>>>>> > * An application should not extend the Node class
>>>>> directly. Doing so may lead to
>>>>> > * an UnsupportedOperationException being thrown.
>>>>> >
>>>>> > Currently this is enforced at runtime in NodeHelper.
>>>>> >
>>>>> > --John
>>>>> >
>>>>> > On 01/02/2023 15:47, Thiago Milczarek Sayão wrote:
>>>>> >
>>>>> > Hi,
>>>>> >
>>>>> > NodeHelper.java has this:
>>>>> >
>>>>> > throw new UnsupportedOperationException(
>>>>> > "Applications should not extend the "
>>>>> > + nodeType + " class directly.");
>>>>> >
>>>>> >
>>>>> > I think it's replaceable with selead classes. Am I
>>>>> right?
>>>>> >
>>>>> > The benefit will be compile time error instead of
>>>>> runtime.
>>>>> >
>>>>> >
>>>>> > -- Thiago.
>>>>> >
>>>>>
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20230201/9b80d9e1/attachment-0001.htm>
More information about the openjfx-dev
mailing list