Setting graphics of a Labeled does not show the Label correctly
Kevin Rushforth
kevin.rushforth at oracle.com
Thu Dec 1 17:30:23 UTC 2022
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-retn at openjdk.org> on behalf of John
> Hendrikx <john.hendrikx at gmail.com>
> *Date: *Thursday, 2022/12/01 at 09:14
> *To: *Nir Lisker <nlisker at gmail.com>
> *Cc: *openjfx-dev at openjdk.org <openjfx-dev at 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.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/c40ecd24/attachment-0001.htm>
More information about the openjfx-dev
mailing list