Comment #4 on issue 160 by cmal...@pixelzoom.com: endless calls to PSwing's ComponentListener
http://code.google.com/p/piccolo2d/issues/detail?id=160

In this case, I think the cure may be worse than the disease ;-)

I vote for removing the ComponentAdapter that's added in the PSwing constructor, and keeping the setVisible override. The setVisible override seems worthwhile, since it keeps the JComponent's visibility in-sync with the PNode (unless the client calls
JComponent.setVisible).

More thoughts...

Setting visibility a couple of times during scenegraph initialization is something that's easy to do, and not necessarily a client error. It's sometimes intentional, as in our simulations that are driven by a complex physical model that is also initializing itself. I don't think this is something that we can expect clients to avoid, and the consequence seem rather severe and difficult for clients to diagnose.
So the price of this optimization is a little too high.

Without the optimization there is the situation you mentioned previously (invisible PSwing node and visible JComponent or visible PSwing and invisible JComponent).
Overriding setVisible keeps the visibility in-sync if the client uses
PSwing.setVisible, which is arguably what they should be doing after instantiating a PSwing. Calling JComponent.setVisible could be documented as a bad practice in the Javadoc for PSwing's constructor; once a JComponent is wrapped with a PSwing, the scenegraph mechanism (and not Swing) should be used to control visibility. (Child
components of the JComponent can still be make visible/invisible via Swing's
setVisible.)

Will clients still make calls to JComponent.setVisible, and cause the visibility of PSwing and JComponent to be out of sync? Yes, probably, but it's much easier to diagnose. And this situation is also easy to detect programmatically, so consider throwing an exception with a very clear message. Maybe something like this?:

         component.addComponentListener(new ComponentAdapter() {

            /** {...@inheritdoc} */
            public void componentHidden(final ComponentEvent e) {
                if ( e.getVisible() != getVisible() ) {
                    visibilitySyncCheck();
                }
            }

            /** {...@inheritdoc} */
            public void componentShown(final ComponentEvent e) {
                if ( e.getVisible() != getVisible() ) {
                     visibilitySyncCheck();
                }
            }
        });

        private void visibilitySyncCheck() {
            if ( component.isVisible() != getVisible() ) {
throw new IllegalStateException( "Visibility of PSwing and JComponent
is out of sync. Did you call setVisible directly on the JComponent?" );
}           }
        }

Caveat: Might be problems with the above, too. Like what happens if a JComponent is
wrapped by a new PSwing? (And is this handled properly in general?)


--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--
Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en

Reply via email to