Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
Could be I twisted my english too far and it broke. ;-) Larry On 1/18/07, Stefan Steiniger <[EMAIL PROTECTED]> wrote: > This is one of those pieces of code that the more I look at it, the more > I see I don't see. Know what I mean? i dont understand that really but i know that i do not understand.. thats why i am a GIS guy and not a programmer :o) good night, stefan - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ___ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
> This is one of those pieces of code that the more I look at it, the more > I see I don't see. Know what I mean? i dont understand that really but i know that i do not understand.. thats why i am a GIS guy and not a programmer :o) good night, stefan - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ___ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
I know exactly what you mean! I'm sorry I ever selected this bug out of the
tracker. :]
I should have followed the method calls that result from the call to
LayerManager.fireCategoryChanged() farther down the chain than I did.
I would still like to understand how things are working. I will print out
your last e-mail and read over it very carefully. I may have a couple more
questions after that if you are willing to continue this conversation.
Thanks again for all of your help.
SS
On 1/18/07, Larry Becker <[EMAIL PROTECTED]> wrote:
Sunburned,
Well, it calls fireLayerEvent with a new inline Runnable class method
run defined to do layerListener.categoryChanged. Then fireLayerEvent
invokes GUIUtil.invokeOnEventThread which does:
if (SwingUtilities.isEventDispatchThread()) {
r.run();
} else {
SwingUtilities.invokeAndWait(r);
}
So if the current thread is an AWT event dispatching thread, it does run()
otherwise it does invokeAndWait() which blocks until all of the AWT events
have been processed.
Without getting too deep in the woods, it seems like the point is that
there is no iteration going on here. All of the iteration is going on in
the fireCategoryChanged method that you modified which should be in thread.
Also note that Jon prevents new events from firing while processing current
events with the isFiringEvents() check.
This is one of those pieces of code that the more I look at it, the more I
see I don't see. Know what I mean?
Larry
On 1/18/07, Sunburned Surveyor <[EMAIL PROTECTED]> wrote:
>
> Larry,
>
> You could be right. I didn't even think about the iteration occuring in
> the GUI Thread. Does this mean that the Runnable argument passed to the
> LayerManager.fireCategoryChanged() method is in fact the GUI Thread? It
> looked like OpenJUMP/JUMP was creating a brand new thread using an inner
> class. (I must admit that I find inner classes almost as confusing as
> threads.)
>
> I'm not disagreeing with your analysis, I'm just trying to understand.
> :]
>
> Here is the code for the LayerManager.fireCategoryChanged() method that
> I thought created a new thread object to iterate over the layerListeners
> collection:
>
>
> fireLayerEvent(new Runnable()
>
> {
>
> public void run()
>
> {
>
> layerListener.categoryChanged(new CategoryEvent(
>
> category, type, categoryIndex));
>
> }
>
>
>
> Thanks for your help Larry.
>
> The Sunburned Surveyor
>
> On 1/18/07, Larry Becker < [EMAIL PROTECTED]> wrote:
>
> > Sunburned,
> >
> > I respectfully disagree with your analysis. I believe all of the
> > iteration over LayerListeners occurs within the GUI Thread. My guess is that
> > although addCategory took the bullet, it didn't fire the gun. Having said
> > that, and looking at the trace, and your fix, I don't see any harm in trying
> > it. I'm a belt and suspenders kind of guy 8-) and I don't have a better
> > answer.
> >
> > This is a pretty old bug (May) and a lot of stuff has changed since
> > then so it may not be possible to reproduce it any more. I tried loading a
> > task with dozens of categories with no luck, even with OJ 1.0.1.
> >
> > regards,
> > Larry
> >
> > On 1/18/07, Sunburned Surveyor < [EMAIL PROTECTED]> wrote:
> >
> > > I have attached the Java file for the modified LayerManager class
> > > that fixes Pedro's bug with the ConcurrentModificationException. All of
the
> > > changes were made to the LayerManager.fireCategoryChanged () method.
> > > I implemented the solution recommended by David, which is to clone the
> > > layerListener collection before firing the event to the listeners. The
> > > LayerManager class was taken from the OpenJUMP CVS.
> > >
> > > This has not been tested in an OpenJUMP build yet. I will test it
> > > with the other bug fixes I need to make for the 01.00.02 release.
> > >
> > > The Sunburned Surveyor
> > >
> > >
> > > -
> > > Take Surveys. Earn Cash. Influence the Future of IT
> > > Join SourceForge.net 's Techsay panel and you'll get the chance to
> > > share your
> > > opinions on IT & business topics through brief surveys - and earn
> > > cash
> > >
> > > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> > >
> > > ___
> > > Jump-pilot-devel mailing list
> > > Jump-pilot-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
> > >
> > >
> > >
> > >
> >
> >
> > -
> > Take Surveys. Earn Cash. Influence the Future of IT
> > Join SourceForge.net's Techsay panel and you'll get the chance to
> > share your
> > opinions on IT & business topics through brief surveys - and earn cash
> >
> > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> >
> > ___
Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
Sunburned,
Well, it calls fireLayerEvent with a new inline Runnable class method run
defined to do layerListener.categoryChanged. Then fireLayerEvent invokes
GUIUtil.invokeOnEventThread which does:
if (SwingUtilities.isEventDispatchThread()) {
r.run();
} else {
SwingUtilities.invokeAndWait(r);
}
So if the current thread is an AWT event dispatching thread, it does run()
otherwise it does invokeAndWait() which blocks until all of the AWT events
have been processed.
Without getting too deep in the woods, it seems like the point is that there
is no iteration going on here. All of the iteration is going on in the
fireCategoryChanged method that you modified which should be in thread.
Also note that Jon prevents new events from firing while processing current
events with the isFiringEvents() check.
This is one of those pieces of code that the more I look at it, the more I
see I don't see. Know what I mean?
Larry
On 1/18/07, Sunburned Surveyor <[EMAIL PROTECTED]> wrote:
Larry,
You could be right. I didn't even think about the iteration occuring in
the GUI Thread. Does this mean that the Runnable argument passed to the
LayerManager.fireCategoryChanged() method is in fact the GUI Thread? It
looked like OpenJUMP/JUMP was creating a brand new thread using an inner
class. (I must admit that I find inner classes almost as confusing as
threads.)
I'm not disagreeing with your analysis, I'm just trying to understand. :]
Here is the code for the LayerManager.fireCategoryChanged() method that I
thought created a new thread object to iterate over the layerListeners
collection:
fireLayerEvent(new Runnable()
{
public void run()
{
layerListener.categoryChanged(new CategoryEvent(
category, type, categoryIndex));
}
Thanks for your help Larry.
The Sunburned Surveyor
On 1/18/07, Larry Becker <[EMAIL PROTECTED]> wrote:
> Sunburned,
>
> I respectfully disagree with your analysis. I believe all of the
> iteration over LayerListeners occurs within the GUI Thread. My guess is that
> although addCategory took the bullet, it didn't fire the gun. Having said
> that, and looking at the trace, and your fix, I don't see any harm in trying
> it. I'm a belt and suspenders kind of guy 8-) and I don't have a better
> answer.
>
> This is a pretty old bug (May) and a lot of stuff has changed since
> then so it may not be possible to reproduce it any more. I tried loading a
> task with dozens of categories with no luck, even with OJ 1.0.1.
>
> regards,
> Larry
>
> On 1/18/07, Sunburned Surveyor < [EMAIL PROTECTED]> wrote:
>
> > I have attached the Java file for the modified LayerManager class that
> > fixes Pedro's bug with the ConcurrentModificationException. All of the
> > changes were made to the LayerManager.fireCategoryChanged () method. I
> > implemented the solution recommended by David, which is to clone the
> > layerListener collection before firing the event to the listeners. The
> > LayerManager class was taken from the OpenJUMP CVS.
> >
> > This has not been tested in an OpenJUMP build yet. I will test it with
> > the other bug fixes I need to make for the 01.00.02 release.
> >
> > The Sunburned Surveyor
> >
> >
> > -
> > Take Surveys. Earn Cash. Influence the Future of IT
> > Join SourceForge.net's Techsay panel and you'll get the chance to
> > share your
> > opinions on IT & business topics through brief surveys - and earn cash
> >
> > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> >
> > ___
> > Jump-pilot-devel mailing list
> > Jump-pilot-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
> >
> >
> >
> >
>
>
> -
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share
> your
> opinions on IT & business topics through brief surveys - and earn cash
>
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>
> ___
> Jump-pilot-devel mailing list
> Jump-pilot-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
>
>
>
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share
your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
___
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
-
Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
Larry,
You could be right. I didn't even think about the iteration occuring in the
GUI Thread. Does this mean that the Runnable argument passed to the
LayerManager.fireCategoryChanged() method is in fact the GUI Thread? It
looked like OpenJUMP/JUMP was creating a brand new thread using an inner
class. (I must admit that I find inner classes almost as confusing as
threads.)
I'm not disagreeing with your analysis, I'm just trying to understand. :]
Here is the code for the LayerManager.fireCategoryChanged() method that I
thought created a new thread object to iterate over the layerListeners
collection:
fireLayerEvent(new Runnable()
{
public void run()
{
layerListener.categoryChanged(new CategoryEvent(
category, type, categoryIndex));
}
Thanks for your help Larry.
The Sunburned Surveyor
On 1/18/07, Larry Becker <[EMAIL PROTECTED]> wrote:
Sunburned,
I respectfully disagree with your analysis. I believe all of the
iteration over LayerListeners occurs within the GUI Thread. My guess is that
although addCategory took the bullet, it didn't fire the gun. Having said
that, and looking at the trace, and your fix, I don't see any harm in trying
it. I'm a belt and suspenders kind of guy 8-) and I don't have a better
answer.
This is a pretty old bug (May) and a lot of stuff has changed since then
so it may not be possible to reproduce it any more. I tried loading a task
with dozens of categories with no luck, even with OJ 1.0.1.
regards,
Larry
On 1/18/07, Sunburned Surveyor <[EMAIL PROTECTED]> wrote:
> I have attached the Java file for the modified LayerManager class that
> fixes Pedro's bug with the ConcurrentModificationException. All of the
> changes were made to the LayerManager.fireCategoryChanged() method. I
> implemented the solution recommended by David, which is to clone the
> layerListener collection before firing the event to the listeners. The
> LayerManager class was taken from the OpenJUMP CVS.
>
> This has not been tested in an OpenJUMP build yet. I will test it with
> the other bug fixes I need to make for the 01.00.02 release.
>
> The Sunburned Surveyor
>
>
> -
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share
> your
> opinions on IT & business topics through brief surveys - and earn cash
>
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>
> ___
> Jump-pilot-devel mailing list
> Jump-pilot-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
>
>
>
>
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share
your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
___
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
Re: [JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
Sunburned, I respectfully disagree with your analysis. I believe all of the iteration over LayerListeners occurs within the GUI Thread. My guess is that although addCategory took the bullet, it didn't fire the gun. Having said that, and looking at the trace, and your fix, I don't see any harm in trying it. I'm a belt and suspenders kind of guy 8-) and I don't have a better answer. This is a pretty old bug (May) and a lot of stuff has changed since then so it may not be possible to reproduce it any more. I tried loading a task with dozens of categories with no luck, even with OJ 1.0.1. regards, Larry On 1/18/07, Sunburned Surveyor <[EMAIL PROTECTED]> wrote: I have attached the Java file for the modified LayerManager class that fixes Pedro's bug with the ConcurrentModificationException. All of the changes were made to the LayerManager.fireCategoryChanged() method. I implemented the solution recommended by David, which is to clone the layerListener collection before firing the event to the listeners. The LayerManager class was taken from the OpenJUMP CVS. This has not been tested in an OpenJUMP build yet. I will test it with the other bug fixes I need to make for the 01.00.02 release. The Sunburned Surveyor - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ___ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
[JPP-Devel] Modified LayerManager class that fixes Bug #1487099...
I have attached the Java file for the modified LayerManager class that fixes
Pedro's bug with the ConcurrentModificationException. All of the changes
were made to the LayerManager.fireCategoryChanged() method. I implemented
the solution recommended by David, which is to clone the layerListener
collection before firing the event to the listeners. The LayerManager class
was taken from the OpenJUMP CVS.
This has not been tested in an OpenJUMP build yet. I will test it with the
other bug fixes I need to make for the 01.00.02 release.
The Sunburned Surveyor
/*
* The Unified Mapping Platform (JUMP) is an extensible, interactive GUI
* for visualizing and manipulating spatial features with geometry and attributes.
*
* Copyright (C) 2003 Vivid Solutions
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
*
* For more information, contact:
*
* Vivid Solutions
* Suite #1A
* 2328 Government Street
* Victoria BC V8T 5G5
* Canada
*
* (250)385-6040
* www.vividsolutions.com
*/
package com.vividsolutions.jump.workbench.model;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.GeometryFactory;
import com.vividsolutions.jts.io.ParseException;
import com.vividsolutions.jts.io.WKTReader;
import com.vividsolutions.jts.util.Assert;
import com.vividsolutions.jump.coordsys.CoordinateSystem;
import com.vividsolutions.jump.coordsys.Reprojector;
import com.vividsolutions.jump.feature.Feature;
import com.vividsolutions.jump.feature.FeatureCollection;
import com.vividsolutions.jump.util.Blackboard;
import com.vividsolutions.jump.util.Block;
import com.vividsolutions.jump.workbench.ui.GUIUtil;
import com.vividsolutions.jump.workbench.ui.renderer.style.BasicStyle;
import com.vividsolutions.jump.workbench.ui.style.AbstractPalettePanel;
import java.awt.Color;
import java.awt.geom.Line2D;
import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationTargetException;
import java.util.*;
/**
* Registry of Layers in a Task.
* @see Task
* @see Layer
*/
public class LayerManager {
private static int layerManagerCount = 0;
private UndoableEditReceiver undoableEditReceiver = new UndoableEditReceiver();
private CoordinateSystem coordinateSystem = CoordinateSystem.UNSPECIFIED;
//As we introduce threaded drawing and other threaded tasks to the
//workbench, we should be careful not to create situations in which
//ConcurrentModificationExceptions can occur. [Jon Aquino]
//Go with List rather than name-to-category map, because category names can
//now change. [Jon Aquino]
private ArrayList categories = new ArrayList();
//Store weak references to layers rather than the layers themselves -- if a layer
//is lucky enough to have all its strong references released, let it dispose of
//itself immediately [Jon Aquino]
private ArrayList layerReferencesToDispose = new ArrayList();
private boolean firingEvents = true;
private ArrayList layerListeners = new ArrayList();
private Iterator firstColors;
private Blackboard blackboard = new Blackboard();
public LayerManager() {
firstColors = firstColors().iterator();
layerManagerCount++;
}
public UndoableEditReceiver getUndoableEditReceiver() {
return undoableEditReceiver;
}
public void deferFiringEvents(Runnable r) {
boolean firingEvents = isFiringEvents();
setFiringEvents(false);
try {
r.run();
} finally {
setFiringEvents(firingEvents);
}
}
private Collection firstColors() {
ArrayList firstColors = new ArrayList();
for (Iterator i = AbstractPalettePanel.basicStyles().iterator();
i.hasNext();) {
BasicStyle basicStyle = (BasicStyle) i.next();
if (!basicStyle.isRenderingFill()) {
continue;
}
firstColors.add(basicStyle.getFillColor());
}
return firstColors;
}
public Color generateLayerFillColor() {
//<> Ensure that colour is not being used by another layer [Jon Aquino]
Color color = firstColors.hasNext() ? (Color) firstColors.next()
: new Color((int) Math.floor(
Math.random() * 256),
(int) Mat
