I'm not sure wrapping it in a runLater would make much difference, and if it does, it would be fragile.

I think that the best approach might be to disallow using the same Node as the "graphic" of two controls, in the same way that we disallow setting a Node as a clip of two different nodes. This would involve a CSR, since it's both a spec and a behavior change. The implementation would need to check whether the Node is in use, and if so, throw an exception (after first unbinding if bound).

-- Kevin


On 12/1/2022 9:20 AM, Andy Goryachev wrote:

Does wrapping the action code in runLater() help?

b1.setOnAction((ev) -> {

Platform.runLater(() -> {
  if (b1.getParent() == cb1) {

...


-andy

*From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of John Hendrikx <john.hendr...@gmail.com>
*Date: *Thursday, 2022/12/01 at 09:14
*To: *Nir Lisker <nlis...@gmail.com>
*Cc: *openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
*Subject: *Re: Setting graphics of a Labeled does not show the Label correctly

The mechanism does seem like it is a bit poorly designed, as it is easy to create inconsistencies.

Luckily it seems that you can't remove a graphic yourself from a Control (getChildren is protected).

I don't think there is an easy solution though...

I think that once you set a graphic on a Labeled, you need to somehow mark it as "in use".  Normally you could just check parent != null for this, but it is trickier than that.  The Skin ultimately determines if it adds the graphic as child, which may be delayed or may even be disabled (content display property is set to showing TEXT only).

Perhaps Skins should always add the graphic and just hide it (visible false, managed false), but that's going to be hard to enforce.

Marking the graphic as "in use" could be done with:

- a property in `getProperties`
- a new "owner" or "ownedBy" property
- allowing "parent" to be set without adding it to children (probably going to mess up stuff)

--John

On 01/12/2022 15:21, Nir Lisker wrote:

    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.

Even if the Labeled is part of the Scene, the graphic may not be (depending on Skin used and on Content Display property for the default skin).

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

Reply via email to