Bugzilla – Full Text Bug Listing
| Summary: | New ClippingStrategy in ConnectionLayer breaks Save As Image | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | <h1055071> |
| Component: | GEF-Legacy Draw2d | Assignee: | Alexander Nyßen <nyssen> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | CC: | ahunter.eclipse, nyssen |
| Version: | 3.6 | ||
| Target Milestone: | 3.6.0 (Helios) M7 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
Caused by Bug #195527 I think the problem may be related to the fact that the newly introduced clipping strategy within connection layer is implemented to clip connections at each viewport within hierarchy of the source and target figure, including the top-most one as well. If this is the case, a solution would probably be to clip connections only at all viewports nested in the primary layer below the first common one of primary and connection layer. I will investigate this. We will need to consider turning the feature off by default and add an API to explicitly turn on the new feature for clients that wish to adopt. We cannot break existing GEF clients. Perhaps a combination of Comment 2 and Comment 3? If it's turned on, then the image saving is still no good. What struck me as inconvenient is the way this has been implemented - a package private interface (IClippingStrategy) and protected methods in the Figure class. After my detective work led me to what was causing the curtailed connections in saved images I thought "OK, let's save the state of the ClippingStrategy, save the image and then restore the ClippingStrategy. But I couldn't do that. In the end, as a workaround, I had to sub-class ScalableFreeformRootEditPart like this: public class HackedScalableFreeformRootEditPart extends ScalableFreeformRootEditPart { class MyConnectionLayer extends ConnectionLayer { MyConnectionLayer() { setClippingStrategy(null); } } @Override protected LayeredPane createPrintableLayers() { LayeredPane layeredPane = super.createPrintableLayers(); layeredPane.removeLayer(CONNECTION_LAYER); layeredPane.add(new MyConnectionLayer(), CONNECTION_LAYER); return layeredPane; } } Well, I do not think we will have to disable the feature by default. As I said, I am pretty much sure that the problem is related to the fact that the current strategy also clips at the uppermost viewport. I am thus quite optimistic that I can fix this for 3.6M7. Furthermore, I think that the clipping problem w.r.t. to nested viewports is a long known problem, and I think we should come up with a decent solution for this, as a lot of clients are waiting for a proper fix. I however share the opinion that it could be a good idea to broaden the visibility of both the interface, as well as the default implementations, so that clients would always have the fallback solution of replacing the viewport-aware clipping strategy with the default clipping strategy and thereby restore the pre 3.6 clipping behavior. I have prototypically implemented a "Save As Image" action for the Logic example editor, and I can reproduce the problem there. While doing so, I have further observed that there are additional (though probably related) clipping problems within the overview thumbnail (when being zoomed) as well as within the editor, when the workbench window (not the editor) is resized and shows scrollbars; the print action however seems to not be affected. As such I do not think that clipping at the uppermost viewport is the (only) problem and I will have to dig a bit deeper... Anthony, concerning your comment, let me say that I of course absolutely share the opinion that we may not break any pre-3.5 clients by this. As such I would propose the following procedure: 1) I will investigate and see if I can come up with a thorough solution to this regression until 3.6M7. 2) If this is not achievable, we can - as ultima ratio - simply comment the setClippingStrategy(...) within the ConnectionLayer constructor and thus restore the old clipping behavior by default (we could then simply re-activate it as soon as the problem gets fixed). Despite, we could discuss whether we want to broaden visibility of IClippingStrategy and its implementations, so clients could make use of it not only via subclassing. Alexander - yes I too get the clipping problems within the overview thumbnail with my application. I think your proposal is sensible and I agree about making the API more visible so clients can either go with the new strategy, provide their own, or default to 3.5 behaviour. I have re-implemented the clipping strategy within ConnectionLayer (renamed it to ConnectionLayerClippingStrategy) so that it does not clip child figures at the root viewport any more. I also broadened its visibility to public and changed the visibility of IClippingStrategy as well as that of related getter/setter method within Figure to public as well, so clients can exchange clipping strategy without overwriting. The new ConnectionLayerClippingStrategy default implementation a bit more "conservative" in a sense that the connection is completely clipped in case source or target are not visible within the common enclosing viewport. This solves some clipping problems that could still be observed with the previous implementation. I added a TODO comment to mark that it is possible to come up with a more decent handling for these cases in the future, so that the connection would not have to be completely clipped. I verified in the logic example editor that saving an image should now work as in pre-3.6 versions. Philip, could you please confirm that your issue is resolved with the current HEAD version as well? (In reply to comment #8) > I have re-implemented the clipping strategy within ConnectionLayer (renamed it > to ConnectionLayerClippingStrategy) so that it does not clip child figures at > the root viewport any more. I also broadened its visibility to public and > changed the visibility of IClippingStrategy as well as that of related > getter/setter method within Figure to public as well, so clients can exchange > clipping strategy without overwriting. > > The new ConnectionLayerClippingStrategy default implementation a bit more > "conservative" in a sense that the connection is completely clipped in case > source or target are not visible within the common enclosing viewport. This > solves some clipping problems that could still be observed with the previous > implementation. I added a TODO comment to mark that it is possible to come up > with a more decent handling for these cases in the future, so that the > connection would not have to be completely clipped. > > I verified in the logic example editor that saving an image should now work as > in pre-3.6 versions. Philip, could you please confirm that your issue is > resolved with the current HEAD version as well? Sure, when is it committed to HEAD and available in an integration build? It is committed to HEAD, so you may retrieve it from this night's integration build (I201004252051). (In reply to comment #10) > It is committed to HEAD, so you may retrieve it from this night's integration > build (I201004252051). http://modeling.eclipse.org/gef/downloads/?showAll=1&hlbuild=I201004252051&sortBy=date&project=#I201004252051 Anthony - thanks for the link, I wouldn't have found it otherwise. Alexander - your changes seem very sensible. I built my app against the latest build and I ran a quick test. The Thumbnail overview displayed OK and the exported image had all of its connections OK. This was a very quick test so that I could feed back to you. I'll keep testing. But so far it looks good. Well done! Phil As there seems to have been no further problem, closing this one as fixed. Please feel free to re-open in case additional problems are observed. |
Build Identifier: GEF 3.6 In GEF 3.6 the ConnectionLayer.java class has a new ClippingStrategy added. The Javadoc comment is as follows: /** * Clipping strategy for connection layer, which takes into account nested * view ports and truncates those parts of connections which reach outside. * * @author Alexander Nyssen * @author Philip Ritzkopf * * @since 3.6 */ Bad news - this means that when I have a GEF Editor diagram that is too big for the Viewport and needs Scrollbars then any connections are cut short in the resulting saved image. I save my image as follows: GraphicalViewer diagramViewer; File file; ... ScalableFreeformRootEditPart rootEditPart = (ScalableFreeformRootEditPart)diagramViewer.getRootEditPart(); IFigure figure = rootEditPart.getLayer(LayerConstants.PRINTABLE_LAYERS); Rectangle rectangle = figure.getBounds(); Image image = new Image(Display.getDefault(), rectangle.width + 50, rectangle.height + 50); GC gc = new GC(image); SWTGraphics graphics = new SWTGraphics(gc); figure.paint(graphics); ImageLoader loader = new ImageLoader(); loader.data = new ImageData[]{image.getImageData()}; loader.save(file, SWT.IMAGE_PNG); ... Reproducible: Always