Bug 279728 - [Layout] Connectors to SVG shapes go through shapes with zoom != 100%
Summary: [Layout] Connectors to SVG shapes go through shapes with zoom != 100%
Status: RESOLVED FIXED
Alias: None
Product: GMF-Runtime
Classification: Modeling
Component: General (show other bugs)
Version: 2.1   Edit
Hardware: All All
: P3 normal
Target Milestone: 2.3   Edit
Assignee: Alex Boyko CLA
QA Contact:
URL:
Whiteboard: 2.1.4 patch
Keywords:
Depends on: 279825
Blocks:
  Show dependency tree
 
Reported: 2009-06-09 22:34 EDT by Alex Boyko CLA
Modified: 2010-07-19 12:28 EDT (History)
3 users (show)

See Also:


Attachments
2.1.4 patch (1.72 KB, patch)
2009-06-09 22:40 EDT, Alex Boyko CLA
no flags Details | Diff
2.1.4 patch with a null check (2.50 KB, patch)
2009-06-10 01:21 EDT, Alex Boyko CLA
no flags Details | Diff
2.2.1 proposed patch (4.96 KB, patch)
2009-06-10 14:53 EDT, Alex Boyko CLA
no flags Details | Diff
patch to address API Tooling issues (4.12 KB, patch)
2010-04-29 14:09 EDT, Alex Boyko CLA
no flags Details | Diff
patch to fix API Tooling issues (4.63 KB, patch)
2010-04-29 17:01 EDT, Alex Boyko CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Boyko CLA 2009-06-09 22:34:41 EDT
1. Create a connector between 2 SVG shapes (100% zoom)
2. Change zoom factor to something != 100%

Connectors either not touching the shapes or going through them.

Problem: Anchor is calculated before ScalableImageFigure either sets the new RenderedImage or renders the image on a non-ui thread.
Need mechanism to let the anchor that the image has been rendered and location needs to be updated.
Comment 1 Alex Boyko CLA 2009-06-09 22:40:43 EDT
Created attachment 138739 [details]
2.1.4 patch

Clients will need to create SlidableImageAnchor as follows:

at the moment the constructor takes two figures:
IFigure owner - a NodeFigure most of the time
ImageFigure imageFigure - the image figure

now, to benefit from the fix they would have to create using the image figure for both the owner and the imageFigure.
For example:
Before: new SlidableImageFigure(owner, imageFigure)
After: new SlidableImageFigure(imageFigure, imageFigure)
Comment 2 Alex Boyko CLA 2009-06-10 01:21:05 EDT
Created attachment 138750 [details]
2.1.4 patch with a null check

Corrected patch (null check added)
Comment 3 Alex Boyko CLA 2009-06-10 14:53:21 EDT
Created attachment 138834 [details]
2.2.1 proposed patch

Patch for 2.2.1
Comment 4 Alex Boyko CLA 2009-06-10 17:25:31 EDT
Hi Cherie,
Could you please code review the 2.1.4 patch for me please?
Thanks.
Comment 5 Alex Boyko CLA 2009-06-11 16:03:30 EDT
2.1.4 patch is committed
Comment 6 Anthony Hunter CLA 2009-08-12 15:40:54 EDT
(In reply to comment #5)
> 2.1.4 patch is committed

Pretty sure we also committed to R2_2_maintenance and HEAD right? So this one is resolved fixed?

Comment 7 Alex Boyko CLA 2009-08-12 15:53:33 EDT
Nope, just the 2.1.4 fix is there. HEAD and 2.2 maintenance are without the fix.
Need a GEF fix to commit this one. It introduces new API, which I think would make the fix correct, not a 2.1.4 workaround with no API changes.
Comment 8 Anthony Hunter CLA 2010-02-04 19:59:52 EST
Ah, this is why you needed the GEF fix.

We still need both right?
Comment 9 Alex Boyko CLA 2010-02-04 22:56:20 EST
Yes, I forgot about this fix. So, yes, we do need a GEF fix to commit it. I'll remind Cherie to review the GEF fix.
Comment 10 Alex Boyko CLA 2010-02-23 17:07:59 EST
Hi James,

Could you please code review this GMF patch as well as the GEF patch attached to the bug 279825? (this patch depends on GEF patch)
Thanks!
Comment 11 James Bruck CLA 2010-02-23 18:08:58 EST
Looks good!  Only minor thing would be to update the Copyright to 2010.
Comment 12 Alex Boyko CLA 2010-02-24 12:12:39 EST
Committed patch has
1. Corrected header dates and @since 3.6
2. Also made all methods AbstractImageFigure final - no need to override add / remove listeners as well notify method.
Comment 13 Alex Boyko CLA 2010-02-24 12:13:45 EST
(In reply to comment #12)
> Committed patch has
> 1. Corrected header dates and @since 3.6
> 2. Also made all methods AbstractImageFigure final - no need to override add /
> remove listeners as well notify method.

Meant to put that in bug 279825
Comment 14 Alex Boyko CLA 2010-04-28 11:03:42 EDT
Delivered the GMF 2.3 fix.
Comment 15 Anthony Hunter CLA 2010-04-28 19:39:44 EDT
API Tools is reporting two API breakages and three missing @since tags in SlidableImageAnchor. The changes need to be done in an API compatible way.

Missing @since tag on imageChanged()
Missing @since tag on SlidableImageAnchor(IFigure, IImageFigure, PrecisionPoint)
Missing @since tag on SlidableImageAnchor(IFigure, IImageFigure)

The constructor org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure, ImageFigure, PrecisionPoint) has been removed
The constructor org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure, ImageFigure) has been removed
Comment 16 Alex Boyko CLA 2010-04-29 10:12:35 EDT
:-( I'll definitely correct this. Thanks!
Comment 17 Alex Boyko CLA 2010-04-29 14:09:20 EDT
Created attachment 166538 [details]
patch to address API Tooling issues

Anthony,

This is the patch to address API tooling problems for gef.ui plugin:
Things done:
1. Upgraded draw2d plug-in dependency version from 3.5.0 to 3.6.0
2. Changed the plug-in version from 1.2.0 to 1.4.0 (skipping 1.3.0, we're making all GMF runtime plugins 1.4.0 for the 2.3, right?)
3. Fixed all API Tooling errors by adding a filter except #imageChanged() method - I added @since for it.

Fixed errors with api tooling filters, because I don't see how I break the public API. API Tooling thinks that I removed old constructors and added the new ones when in fact i simply switched variable type to  IImageFigure from ImageFigure. ImageFigure is a sub-class of IImageFigure. Therefore, I expanded the API, which shouldn't break it.

Let me know what you think.
Comment 18 Anthony Hunter CLA 2010-04-29 15:56:50 EDT
Hi Alex, for the two removed methods:
The constructor
org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure,
ImageFigure, PrecisionPoint) has been removed
The constructor
org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure,
ImageFigure) has been removed

If we add them back and make them deprecated, then we will maintain binary compatibility and API tools will not complain.
Comment 19 Alex Boyko CLA 2010-04-29 17:01:05 EDT
Created attachment 166572 [details]
patch to fix API Tooling issues

Anthony,
Here is the new patch I've created by adding the old constructors back. I didn't have to correct the references to those constructors, they are automatically reference the new ones. Hence, I feel that API Tooling is incorrect in this case and not us.
Let me know if it's ok to commit it.
Comment 20 Alex Boyko CLA 2010-04-29 17:05:07 EDT
Actually, I have to correct references to these constructors if i add the old constructors... by casting ImageFigure to IImageFigure... Wouldn't say I like this.
Comment 21 Anthony Hunter CLA 2010-04-29 20:32:17 EDT
(In reply to comment #18)
> The constructor org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure,
> ImageFigure, PrecisionPoint) has been removed
> The constructor
> org.eclipse.gmf.runtime.gef.ui.figures.SlidableImageAnchor.SlidableImageAnchor(IFigure,
> ImageFigure) has been removed
> 

OK, agreed, just add a .api_filter for these.
Comment 22 Alex Boyko CLA 2010-04-30 11:44:48 EDT
Delivered the previous patch that fixes API Tooling issues via api filters.
Comment 23 Eclipse Webmaster CLA 2010-07-16 23:38:16 EDT
[target cleanup] 2.3 M7 was the original target milestone for this
bug
Comment 24 Eclipse Webmaster CLA 2010-07-19 12:28:20 EDT
[GMF Restructure] Bug 319140 : product GMF and component
Runtime Diagram was the original product and component for this bug