I think this is a case where it is a lot simpler to fix GEF.

That would be to just fix FigEdge.addPathItem to set the path item Fig
to the same layer as the edge.

FigEdge.setLayer will also have to be changed to rattle through the
path items and update those also.

Bob.


2008/11/15 Dave Thompson <[EMAIL PROTECTED]>:
> Hi Bob,
>
> Ok, I have been working on a change to make sure all child Figs within a
> FigEdgeModelElement have the same Layer as the parent fig.  My change is a
> List (figList) of all child figs within FigEdgeModelElement which is
> iterated when setLayer() is called.
>
> When I use this, I don't get any more "layer is null" messages while
> dragging the labels around.  I make the assumption that the constructor will
> define all of the Figs, and setLayer() will be called after the constructor
> or at the end of it (therefore the layer will be set on all child Figs after
> they have been created), and not before.
>
> The only negative impact is that you have to remember to add any new Figs to
> this list when modifying the constructor on FigEdgeModelElement or any of
> it's descendants.  If you forget, it's not serious, it would probably just
> result in the message again.
>
> It seems that 'layer-less' figs are still draggable, it's just that they
> don't update on the screen while being dragged - only when you release the
> mouse button.
>
> The good thing is that the figList then becomes very useful for my work on
> 1048, when scanning through the child figs to find out which one a user
> clicked on.
>
> Shall I commit this change?
>
> Regards,
>
> Dave
>
>
> Bob Tarling wrote:
>>
>> Everyone here is before this design decision in GEF.
>>
>> I think the intention is that it is only that top level figs
>> (FigNodes, FigEdges and the primitive FigText, FigLine etc) are
>> considered to be composite parts of a Layer. The child Figs that then
>> make up the detail of a FigNode of FigEdge are considered composite
>> parts of that Fig and so the layer is redundant.
>>
>> It's a shame 2 different types of composition are being used where
>> only one is relevant at a time. I think I'd have preferred to have a
>> common interface for both FigGroup and Layer and base a parent/child
>> composition on that.
>>
>> However at the moment we are more likely to just enforce that child
>> figs have the same layer setting as their parent. Note though that
>> that should be a one way reference only for child figs. The Layer
>> should only contain a list of Figs at the top level of the fig
>> composition hierarchy.
>>
>> Bob.
>>
>>
>>
>> 2008/11/15 Dave Thompson <[EMAIL PROTECTED]>:
>>>
>>> Hi,
>>>
>>> I've been working on issue 1048, and during this, I found that some of
>>> the
>>> Fig elements on the display have getLayer() == null.  This causes some
>>> problems when trying to do things with text labels, such as move them.
>>>  When
>>> I do start to move them, I get stdout filling up with:
>>>
>>> SelectionManager: layer is null
>>>
>>> Which is coming from org.tigris.gef.base.SelectionManager.drag(int,int)
>>>
>>> I think I've found the reason why this situation occurs, but I'm just
>>> wondering whether it is intentional or not.
>>>
>>> Regards,
>>>
>>> Dave
>>>
>>> ---------------------------------------------------------------------
>>> 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]
>>
>
> Index: src/org/argouml/uml/diagram/sequence/ui/FigMessage.java
> ===================================================================
> --- src/org/argouml/uml/diagram/sequence/ui/FigMessage.java     (revision
> 16024)
> +++ src/org/argouml/uml/diagram/sequence/ui/FigMessage.java     (working
> copy)
> @@ -59,6 +59,7 @@
>         textGroup.addFig(getStereotypeFig());
>         addPathItem(textGroup, new PathConvPercent(this, 50, 10));
>         setOwner(owner);
> +        addToFigList(textGroup);
>     }
>
>     /**
> Index: src/org/argouml/uml/diagram/ui/FigAssociation.java
> ===================================================================
> --- src/org/argouml/uml/diagram/ui/FigAssociation.java  (revision 16024)
> +++ src/org/argouml/uml/diagram/ui/FigAssociation.java  (working copy)
> @@ -124,6 +124,13 @@
>         addPathItem(destGroup,
>                    new PathConvPercentPlusConst(this, 100, -35, -15));
>
> +        // Update the local list of figs.
> +        addToFigList(srcGroup);
> +        addToFigList(middleGroup);
> +        addToFigList(destGroup);
> +        addToFigList(srcMult);
> +        addToFigList(destMult);
> +
>         setBetweenNearestPoints(true);
>
>         // next line necessary for loading
> Index: src/org/argouml/uml/diagram/ui/FigAssociationEnd.java
> ===================================================================
> --- src/org/argouml/uml/diagram/ui/FigAssociationEnd.java       (revision
> 16024)
> +++ src/org/argouml/uml/diagram/ui/FigAssociationEnd.java       (working
> copy)
> @@ -100,6 +100,9 @@
>         addPathItem(srcMult, new PathConvPercentPlusConst(this, 100, -15,
> -15));
>         addPathItem(srcGroup, new PathConvPercentPlusConst(this, 100, -40,
> 20));
>
> +        addToFigList(srcMult);
> +        addToFigList(srcOrdering);
> +        addToFigList(srcGroup);
>         setBetweenNearestPoints(true);
>         // next line necessary for loading
>         setLayer(ProjectManager.getManager().getCurrentProject()
> Index: src/org/argouml/uml/diagram/ui/FigDependency.java
> ===================================================================
> --- src/org/argouml/uml/diagram/ui/FigDependency.java   (revision 16024)
> +++ src/org/argouml/uml/diagram/ui/FigDependency.java   (working copy)
> @@ -63,6 +63,7 @@
>         endArrow = new ArrowHeadGreater();
>         endArrow.setFillColor(Color.red);
>         setDestArrowHead(endArrow);
> +        addToFigList(middleGroup);
>         setBetweenNearestPoints(true);
>         setLayer(ProjectManager.getManager()
>                 .getCurrentProject().getActiveDiagram().getLayer());
> Index: src/org/argouml/uml/diagram/ui/FigEdgeModelElement.java
> ===================================================================
> --- src/org/argouml/uml/diagram/ui/FigEdgeModelElement.java     (revision
> 16024)
> +++ src/org/argouml/uml/diagram/ui/FigEdgeModelElement.java     (working
> copy)
> @@ -36,6 +36,7 @@
>  import java.beans.PropertyChangeEvent;
>  import java.beans.PropertyChangeListener;
>  import java.beans.VetoableChangeListener;
> +import java.util.ArrayList;
>  import java.util.HashMap;
>  import java.util.HashSet;
>  import java.util.Iterator;
> @@ -140,6 +141,12 @@
>     private HashMap<String, Object> npArguments = new HashMap<String,
> Object>();
>
>     /**
> +     * A local list of all sub-figs within the FigEdgeModelElement, to help
> +     * with iterating, etc.
> +     */
> +    private List<Fig> figList = new ArrayList<Fig>();
> +
> +    /**
>      * The Fig that displays the name of this model element.
>      * Use getNameFig(), no setter should be required.
>      */
> @@ -176,6 +183,10 @@
>
>         setBetweenNearestPoints(true);
>
> +        // Keep an updated local list of all figs, to help with iterating,
> etc.
> +        addToFigList(nameFig);
> +        addToFigList(stereotypeFig);
> +
>         ArgoEventPump.addListener(ArgoEventTypes.ANY_NOTATION_EVENT, this);
>         ArgoEventPump.addListener(
>                 ArgoEventTypes.ANY_DIAGRAM_APPEARANCE_EVENT, this);
> @@ -192,6 +203,21 @@
>     }
>
>     /**
> +     * Adds the specified fig to the internal fig list.
> +     * If fig is a FigGroup, each internal fig is added seperately.
> +     * @param fig The Fig or FigGroup to add.
> +     */
> +    protected void addToFigList(Fig fig) {
> +        if (fig instanceof FigGroup) {
> +            figList.addAll(((FigGroup)fig).getFigs());
> +        }
> +        else {
> +            figList.add(fig);
> +        }
> +
> +    }
> +
> +    /**
>      * Create a FigCommentPort if needed
>      */
>     public void makeEdgePort() {
> @@ -983,9 +1009,8 @@
>         super.setLayer(lay);
>         getFig().setLayer(lay);
>
> -        // TODO: Workaround for GEF redraw problem
> -        // Force all child figs into the same layer
> -        for (Fig f : (List<Fig>) getPathItemFigs()) {
> +        // Set all child figs to be on the same layer.
> +        for (Fig f : figList) {
>             f.setLayer(lay);
>         }
>     }
> Index: src/org/argouml/uml/diagram/ui/FigGeneralization.java
> ===================================================================
> --- src/org/argouml/uml/diagram/ui/FigGeneralization.java       (revision
> 16024)
> +++ src/org/argouml/uml/diagram/ui/FigGeneralization.java       (working
> copy)
> @@ -73,6 +73,8 @@
>         endArrow = new ArrowHeadTriangle();
>        endArrow.setFillColor(Color.white);
>        setDestArrowHead(endArrow);
> +
> +        addToFigList(discriminator);
>        setBetweenNearestPoints(true);
>
>        if (getLayer() == null) {
> Index: src/org/argouml/uml/diagram/use_case/ui/FigExtend.java
> ===================================================================
> --- src/org/argouml/uml/diagram/use_case/ui/FigExtend.java      (revision
> 16024)
> +++ src/org/argouml/uml/diagram/use_case/ui/FigExtend.java      (working
> copy)
> @@ -133,8 +133,9 @@
>
>         setDestArrowHead(endArrow);
>
> +        // Update local list of figs.
> +        addToFigList(fg);
>         // Make the edge go between nearest points
> -
>         setBetweenNearestPoints(true);
>     }
>
> Index: src/org/argouml/uml/diagram/use_case/ui/FigInclude.java
> ===================================================================
> --- src/org/argouml/uml/diagram/use_case/ui/FigInclude.java     (revision
> 16024)
> +++ src/org/argouml/uml/diagram/use_case/ui/FigInclude.java     (working
> copy)
> @@ -84,6 +84,9 @@
>         // Add an arrow with an open arrow head
>
>         setDestArrowHead(endArrow);
> +
> +        // Update the local list of figs.
> +        addToFigList(label);
>
>         // Make the edge go between nearest points
>
>
> ---------------------------------------------------------------------
> 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]

Reply via email to