I've found a way to remove the need for UI update in FigClassifierRole.modelChanged.
That doesn't remove the principle of this conversation though. I think modelChanged in other Figs likely do much in the way of UI updates. On 12/04/2008, Michiel van der Wulp <[EMAIL PROTECTED]> wrote: > Hi Christian, > > I had made a thorough study about the listeners to model changes in the > Figs, and amended/created most of this code. > > The call > updateListeners(getOwner(),getOwner()); > in the > org.argouml.uml.diagram.sequence2.ui.FigClassifierRole > class indeed does nothing. > But I suspect that this was copied from the > org.argouml.uml.diagram.sequence.ui.FigClassifierRole > class, where it definitively does a lot! > > Maybe you better leave it in... > > Regards, > Michiel > > > > ----- Original Message ----- From: "Christian López Espínola" > <[EMAIL PROTECTED]> > To: <[email protected]> > Sent: Saturday, April 12, 2008 6:51 PM > Subject: Re: [argouml-dev] modelChanged in Figs should invokeLater > > > > > > Hi all, > > > > As Michiel points, we shouldn't abuse of the AWT thread. > > updateListeners is thread-safe, so it can be called in any thread > > without problems. > > > > I'd like to update Bob's solution as below. > > > > BTW, I agree with Bob that we should investigate about calling > > updateListeners. Maybe I'm missing something, but what is the reason > > of > > updateListeners(getOwner(),getOwner())? In ModelElement, > updateListeners has: > > protected void updateListeners(Object oldOwner, Object newOwner) { > > if (oldOwner == newOwner) { > > return; > > } > > Shouldn't this call be removed or I'm missing something? > > > > > > On Sat, Apr 12, 2008 at 6:05 PM, Bob Tarling <[EMAIL PROTECTED]> > wrote: > > > > > Hmmm, I suspect that we're doing far too much when we get a model > > > event anyway, I've never been sure about things like > > > FigNodeModelElement.updateListeners being called so > frequently. I > > > think that needs some investigation. > > > > > > But if we are aware that there are certain things that should happen > > > in different threads then I think it would be useful to separate > > > methods for handling each thread in a consistent way. > > > > > > e.g. in FigNodeModelElement > > > > > > > > > > > final protected void modelChanged(final PropertyChangeEvent mee) { > > Runnable doWorkRunnable = new Runnable() { > > public void run() { > > modelChangedImpl(mee); > > } > > }; > > SwingUtilities.invokeLater(doWorkRunnable); > > updateNotation(); > > updateListeners(); <<< > > } > > > > > > > > /** > > > * Alert the notation subsystem that change has taken place in the > model > > > */ > > > protected void updateNotation(PropertyChangeEvent > event) { > > > // Here goes the notation code previously in modelChanged > > > } > > > > > > /** > > > * Guaranteed to execute in AWT thread to update the layout of the > > > Fig composition > > > * as a result of a change in the model. > > > */ > > > protected void modelChangedImpl(PropertyChangeEvent > event) { > > > // Here goes the other code previously in modelChanged > > > > > > > > > } > > > > > > > > > > > > > > > On 12/04/2008, Bob Tarling <[EMAIL PROTECTED]> wrote: > > > > Is it not possibel that updateListeners and > > > > NotationProvider.updateListener is ever called from > the AWT thread > > > > also? So we just move the problem somewhere else. > > > > > > > > Is there actually an advantage of having those calls on the MDR > > > > thread? Is it a performance issue to take work away from the AWT > > > > thread? > > > > > > > > If not then wouldn't it be safer to make sure that everything in > that > > > > method is on AWT thread by my suggested change to > > > > FigNodeModelElement/FigEdgeModelElement. > > > > > > > > > > > > Bob, > > > > > > > > > > > > > > > > > > > > On 12/04/2008, Michiel van der Wulp <[EMAIL PROTECTED]> wrote: > > > > > Hi Bob, Christian, > > > > > > > > > > The current code is as follows: > > > > > > > > > > protected void modelChanged(final PropertyChangeEvent mee) { > > > > > super.modelChanged(mee); > > > > > if (mee instanceof AddAssociationEvent > > > > > || mee instanceof RemoveAssociationEvent > > > > > || mee instanceof AttributeChangeEvent) { > > > > > Runnable doWorkRunnable = new Runnable() { > > > > > public void run() { > > > > > renderingChanged(); > > > > > updateListeners(getOwner(), getOwner()); > > > > > > notationProvider.updateListener( > > > > > FigClassifierRole.this, getOwner(), mee); > > > > > damage(); > > > > > } > > > > > }; > > > > > > > > > > > SwingUtilities.invokeLater(doWorkRunnable); > > > > > } > > > > > } > > > > > > > > > > > > > > > But, since the both updateListener calls only are intended to > query the > > > > > model, and do not impact the graphic representation, it should be > as > > > > > follows: > > > > > > > > > > protected void modelChanged(final PropertyChangeEvent mee) { > > > > > super.modelChanged(mee); > > > > > if (mee instanceof AddAssociationEvent > > > > > || mee instanceof RemoveAssociationEvent > > > > > || mee instanceof AttributeChangeEvent) { > > > > > Runnable doWorkRunnable = new Runnable() { > > > > > public void run() { > > > > > renderingChanged(); > > > > > damage(); > > > > > } > > > > > }; > > > > > updateListeners(getOwner(), getOwner()); > > > > > notationProvider.updateListener( > > > > > FigClassifierRole.this, getOwner(), mee); > > > > > > SwingUtilities.invokeLater(doWorkRunnable); > > > > > } > > > > > } > > > > > > > > > > IIRC, then this kind of seperating between threads has been > discussed > > > > > between me and Tom. > > > > > Please also read the comments with > > > > > NotationProvider.updateListener(). > > > > > > > > > > Regards, > > > > > Michiel > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- From: "Bob Tarling" > <[EMAIL PROTECTED]> > > > > > To: <[email protected]> > > > > > Sent: Saturday, April 12, 2008 1:48 AM > > > > > Subject: [argouml-dev] modelChanged in Figs should invokeLater > > > > > > > > > > > > > > > > > > > > > > > > > > > Christian came across a problem with concurrency in > FigClassifierRole > > > > > > of the new sequence diagrams. > > > > > > > > > > > > The problem was that drawing of a message edge (user interaction > an > > > > > > AWT thread) was effecting the Fig composition at the same time > as the > > > > > > same Fig was receiving a model event which also resulted in a > change > > > > > > to the Fig composition. > > > > > > > > > > > > Forcing the handling of the model event to the AWT thread solved > this. > > > > > > > > > > > > I'm thinking that this is probably something we should do in > general > > > > > > in Figs to be safe > > > > > > > > > > > > Something like the following in FigNodeModelElement (and Edge) > > > > > > > > > > > > final protected void modelChanged(final PropertyChangeEvent > mee) { > > > > > > Runnable doWorkRunnable = new Runnable() { > > > > > > public void run() { > > > > > > modelChangedImpl(mee); > > > > > > } > > > > > > }; > > > > > > > > > > > > > SwingUtilities.invokeLater(doWorkRunnable); > > > > > > } > > > > > > > > > > > > protected void > modelChangedImpl(PropertyChangeEvent > > > > > mee) { > > > > > > // Here goes the code previously in modelChanged > > > > > > } > > > > > > > > > > > > > > > > > > Sound reasonable? > > > > > > > > > > > > Bob. > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > To unsubscribe, e-mail: > > > > > [EMAIL PROTECTED] > > > > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > No virus found in this incoming message. > > > > > > Checked by AVG. > > > > > > Version: 7.5.519 / Virus Database: 269.22.12/1373 - Release > Date: > > > > > 11/04/2008 9:17 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: > [EMAIL PROTECTED] > > > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: > [EMAIL PROTECTED] > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > -- > > Regards, > > > > Christian López Espínola > > > > > > > -------------------------------------------------------------------------------- > > > No virus found in this incoming message. > Checked by AVG. > Version: 7.5.519 / Virus Database: 269.22.13/1375 - Release Date: > 12/04/2008 11:32 > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
