Community
Participate
Working Groups
Created attachment 152258 [details] proposed patch 1. IGraphicalEditPart#getDiagramView() implementation on GraphicalEditPart and ConnectionEditPart should use View#getDiagram() instead of assuming Diagram is the model for contents (diagram) editpart 2. IGraphicalEditPart#getPrimaryView() implementation on GraphicalEditPart and ConnectionEditPart should be overridable. It also should delegate finding of primary view to the parent editpart. 3. Currently it is possible to create connection editparts where source or target editpart is never created. I propose that we don't create a connection editpart until its source and traget editpart is created. 4. Some safety fixes for bendpoints and labels
Alex, here are my comments regarding your changes: I think the IGraphicalEditPart#getPrimaryView() implementation on GraphicalEditPart and ConnectionEditPart should also include a check on the first line to make sure the model is in fact a view... if (this instanceof IPrimaryEditPart && getModel() instanceof View) { return (View) getModel(); } Regarding #3, why is this necessary? It seems a bit risky to me to change refreshSourceConnections() and refreshTargetConnections() at this point, especially when the documentation in GEF says "This method should <em>not</em> be overridden.". Is there an alternative to solving your problem?
Thanks for the feedback, Cherie. #2 (Primary view) - Agreed #3 I had a feeling this will be a concern. The problem. Diagram View1 Compartment View2 Edge(View1, View2) If Diagram editpart is created for Compartment and set as contents for the viewer, the editpart for the edge between View1 and View2 would be created, despite the fact that editpart for View1 won't be created. Hence a connection editpart without the source. In editparts it would look like: DiagramEditPart (Compartment) ShapeNodeEditPart (View2) ConnectionEditPart (Edge) The connection would look like a short line segment in the diagram's top left corner. You probably have seen this many times. This happens when connection editpart doesn't have either source or target. There are obviously a couple of alternatives: 1. Don't create connection editparts unless there is a source and target editpart present (Solution from the attached patch) 2. Don't add connection figure to the connection layer unless there is a source and target editpart present for the connection. Requires fix in GEF. If connection is not added to the layer, get a lot of mapmode warnings. Seemed riskier than the first alternative. 3. Don't paint the connection figure that doesn't have source and/or target anchor. Don't paint by returning empty bounds from getBounds() method. It's a workaround - hides the problem. The patch is corrected for #2 and #3 with alternative 3. In addition, ConnectionRefreshMagr will hide connections without source and/or target.
Created attachment 152418 [details] proposed patch another patch for review
I understand. It looks like the AbstractGraphicalEditPart.getModelSourceConnections() method is meant to be used to filter out connections that should not have editparts created for them. We have done something similar in an override of this method in ShapeNodeEditPart where we check if the view is visible. However, I'm guessing there is nothing to tell you based on the target view whether the target will or will not have an editpart. My concern with the first suggested fix is that there is some scenario where the connection editparts get created before the source or target editpart and that is ok. I think your second proposed fix is fine if it solves your problem. It seems less risky to me. We already do something similar in the ConnectionRefreshManager when we check if the target/source figure is showing.
Committed to head and 2_2 maintenance
[GMF Restructure] Bug 319140 : product GMF and component Runtime Diagram was the original product and component for this bug
