Bug 237202 - Finding non-conflicting location of border items not working properly in all scenarios
Summary: Finding non-conflicting location of border items not working properly in all ...
Status: RESOLVED FIXED
Alias: None
Product: GMF-Runtime
Classification: Modeling
Component: General (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows XP
: P3 normal
Target Milestone: 2.1.1   Edit
Assignee: Alex Boyko CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 237203 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-06-14 17:18 EDT by Adam Neal CLA
Modified: 2010-07-19 21:56 EDT (History)
2 users (show)

See Also:


Attachments
Patch to fix problem identified above - created against v20080516_1748 (2.24 KB, patch)
2008-06-14 17:18 EDT, Adam Neal CLA
no flags Details | Diff
alternative patch (4.32 KB, patch)
2008-06-17 16:56 EDT, Alex Boyko CLA
no flags Details | Diff
Small change on Alex's patch (8.27 KB, patch)
2008-06-17 19:53 EDT, Adam Neal CLA
ahunter.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Neal CLA 2008-06-14 17:18:34 EDT
Created attachment 104970 [details]
Patch to fix problem identified above - created against v20080516_1748

Build ID: I20080516-1333

Steps To Reproduce:
In our tooling we have import issues because the size of the border items in our legacy tool are smaller then the size in our current tool.  Therefore users were able to position border items closer together then we allow currently.  In this case the border items, after import, could be 'over lapping' and thus conflicting.  The default behavior of the BorderItemLocator is to check locations in 'diameter' increments (i.e add or subtract the diameter of the border item to find the next possible non-conflicting location ).  However, because other border items may be in a conflicting location after moving that distance, the imported item could be placed way to far away from where it should be.

To fix this , I simply changed the code in locateOnBorder (in the border item locator) to use 1/4 of the diameter as the next point to check.  Using a smaller distance to check for a conflicting location allowed the imported border item to be positioned correctly.

I would like to request that a smaller increment be used in GMF's org.eclipse.gmf.runtime.diagram.ui.figures.BorderItemLocator

More information:
I have set this defect as critical since it has been raised by a customer and is blocking a BP1 for us.
Comment 1 Alex Boyko CLA 2008-06-17 16:56:37 EDT
Created attachment 105212 [details]
alternative patch

Not sure that we need this 0.25 magic scaling factor. The purpose of the collision resolution for border items is to provide horizontal_gap and verical_gap (which is equal to 8 pixels) between consecutive border items. I agree that currently the collision resolution algorithm doesn't satisfy its purpose completely.
I attched the patch that finds the border item with wich our current border items is colliding with. Now to place our border item we offset 8 pixels down from the bottom of the conflicitng border item for the WEST side, similarly for other sides.
Adam, please try this patch and provide feedback.
If the 8 pixels gap is too small/large i could add setters for the gap pixel value.
Comment 2 Adam Neal CLA 2008-06-17 19:53:09 EDT
Created attachment 105229 [details]
Small change on Alex's patch
Comment 3 Adam Neal CLA 2008-06-17 19:55:37 EDT
Hmmm, I had typed out comments, but they didn't seem to get posted with the patch.

Alex, your patch worked well for me.  We have one more issue with the conflicting location algorithm, in that we need control over the direction that the next position is located.  This is the patch that I applied (simply a small change on your patch).

Please review and let me know if you are ok with it, or if you have another proposed solution.

Thanks,
Adam
Comment 4 Alex Boyko CLA 2008-06-17 22:24:37 EDT
Adam, it looks like you guys just want to move the border item in the clockwise direction instead of the counter-clockwise, right?
Personally, I think that this is the only option for this collision resolving algorithm. So, why don't we simply create this option for you. A boolean variable will stand for the direction and will be used internally by the locator. Yet, the variable could be set from the contsructor and/or a setter. This could be a better solution if you don't have to override anything to adopt it. Let me know.
Comment 5 Alex Boyko CLA 2008-06-18 18:36:53 EDT
Fixed in 2.1.1. Latest Adam's patch has been committed.
Comment 6 Anthony Hunter CLA 2008-07-08 16:09:49 EDT
*** Bug 237203 has been marked as a duplicate of this bug. ***
Comment 7 Eclipse Webmaster CLA 2010-07-19 21:56:44 EDT
[GMF Restructure] Bug 319140 : product GMF and component
Runtime was the original product and component for this bug