Setting graphics of a Labeled does not show the Label correctly

[Nir Lisker]

Technically it doesn't appear elsewhere in the scenegraph, it is the child
of only one label. It's set as the graphics property of 2 labels though.
The mismatch is that being set as a graphics property isn't a 1-to-1
relationship with being a child of the label.

Something has to be fixed along this chain of inconsistency, I just wasn't
sure where. It seems like the IAE that you mentioned should be thrown, but
isn't. I can write a PR for that. One thing I'm not sure about is: in a
situation where the graphic belongs to a label that is detached from a
scene, and that graphic is set to a label that *is* part of a scene, should
an IAE be thrown as well.

By the way, changing to throwing an IAE is going to cause some new
exceptions. There is a possibility that some VirtualFlow things will break
because that mechanism recycles nodes and re-attaches their properties.
Then again, it might just mean that it was done wrong.

On Thu, Dec 1, 2022 at 3:49 PM John Hendrikx <john.hendrikx at gmail.com>
wrote:

> Sorry, I meant on the first click already.
>
> --John
> On 01/12/2022 14:46, John Hendrikx wrote:
>
> Setting the same Node for multiple graphics is not allowed.  Your program
> should IMHO throw "IllegalArgumentException" on the 2nd click.
>
>      * An optional icon for the Labeled. This can be positioned relative
> to the
>      * text by using {@link #setContentDisplay}.  The node specified for
> this
>      * variable cannot appear elsewhere in the scene graph, otherwise
>      * the {@code IllegalArgumentException} is thrown.  See the class
>      * description of {@link Node} for more detail.
>
> --John
> On 01/12/2022 13:38, Nir Lisker wrote:
>
> That's my point. Currently, a node can serve as the graphics property for
> multiple Labels, but will appear only in 1 because it can only have a
> single parent in the scenegraph. While this is technically fine, it causes
> issues like the one I showed above. I'm not sure if a node is supposed to
> be able to serve as multiple graphics (and displayed only in the last Label
> it was set to?), or removed from other Labels' graphic properties just like
> is done for children in the scenegraph. Personally, I find it confusing
> that label.getGraphics() will return a node that isn't shown in that label.
>
> On Thu, Dec 1, 2022 at 2:21 PM John Hendrikx <john.hendrikx at gmail.com>
> wrote:
>
>> Internally the graphics is just a child node, and nodes can't be part of
>> the scene graph twice (this is done in LabeledSkinBase).
>>
>> It showing up only once is probably because it is getting removed from
>> the other labels.
>>
>> I think things are probably getting out of sync, where the graphics
>> property may think it still has a certain node as its graphics, but it
>> is no longer a child of the label.
>>
>> --John
>>
>> On 01/12/2022 11:23, Nir Lisker wrote:
>> > Hi,
>> >
>> > Given the following code
>> >
>> >         var cb1 = new Label("1");
>> >         var cb2 = new Label  ("2");
>> >         var b1 = new Button("A");
>> >         cb1.setGraphic(b1);
>> >         b1.setOnAction(e -> {
>> >             if (b1.getParent() == cb1) {
>> >                 // cb1.setGraphic(null);
>> >                 cb2.setGraphic(b1);
>> >             } else {
>> >                 // cb2.setGraphic(null);
>> >                 cb1.setGraphic(b1);
>> >             }
>> >         });
>> >
>> > Pressing the first button will move it (the graphic) to the second
>> > label, however, pressing it again will not move it back to the first
>> > label. It's required to set the graphics to null prior to moving them
>> > as in the commented lines. This looks like a bug to me. I would think
>> > that when a graphic is moved, it will appear in its new label
>> > immediately, like moving a child. Apparently a single node can be the
>> > graphics for multiple Labeled nodes, but it will appear only in 1. Is
>> > this intentional?
>> >
>> > - Nir
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20221201/b612d38c/attachment-0001.htm>