Community
Participate
Working Groups
SnapToGrid has an issue where it only works for snapping to the top right (NORTH | WEST) corner of a figure. The issue is that in the code for handling SOUTH and EAST, it sets preciseHeight and preciseWidth instead of preciseX and preciseY. Since preciseX and preciseY is used to determine snapping, but is always 0, snapping then turns off completely. It's easy to replicate the issue. Simply change snapLocations in SnapToGrid.snapRectangle to (EAST | SOUTH). This should make the figure snap to the bottom right corner, but instead turns all snapping off. The code below fixes the issue: public int snapRectangle(Request request, int snapLocations, PrecisionRectangle rect, PrecisionRectangle result) { rect = rect.getPreciseCopy(); makeRelative(container.getContentPane(), rect); PrecisionRectangle correction = new PrecisionRectangle(); makeRelative(container.getContentPane(), correction); if (gridX > 0 && (snapLocations & EAST) != 0) { double rightCorrection = Math.IEEEremainder(rect.preciseRight() - origin.x - 1, gridX); correction.preciseX -= rightCorrection; snapLocations &= ~EAST; } if ((snapLocations & (WEST | HORIZONTAL)) != 0 && gridX > 0) { double leftCorrection = Math.IEEEremainder(rect.preciseX - origin.x, gridX); correction.preciseX -= leftCorrection; if ((snapLocations & HORIZONTAL) == 0) correction.preciseWidth += leftCorrection; snapLocations &= ~(WEST | HORIZONTAL); } if ((snapLocations & SOUTH) != 0 && gridY > 0) { double bottomCorrection = Math.IEEEremainder(rect.preciseBottom() - origin.y - 1, gridY); correction.preciseY -= bottomCorrection; snapLocations &= ~SOUTH; } if ((snapLocations & (NORTH | VERTICAL)) != 0 && gridY > 0) { double topCorrection = Math.IEEEremainder( rect.preciseY - origin.y, gridY); correction.preciseY -= topCorrection; if ((snapLocations & VERTICAL) == 0) correction.preciseHeight += topCorrection; snapLocations &= ~(NORTH | VERTICAL); } correction.updateInts(); makeAbsolute(container.getContentPane(), correction); result.preciseX += correction.preciseX; result.preciseY += correction.preciseY; result.preciseWidth += correction.preciseWidth; result.preciseHeight += correction.preciseHeight; result.updateInts(); return snapLocations; }
Created attachment 58666 [details] Patch to fix SnapToGrid issue and allow it to snap to EAST and/or SOUTH
Snap to grid is designed to only snap top-left locations when something is being moved (not resized). The current behavior is the correct default behavior and reflects how other tools such as PowerPoint or Visio operate.
Randy, as a default behaviour, it works perfectly and I'm not disputing that. However, if I change the snapLocations, I would expect the code to allow me to snap to different locations. The code as it exists in GEF at the moment only allows top-left. If I change the snap locations, it should be able to snap to different locations. Even though most people uses the top left as a snap point, the code should stils support snapping to the EAST or SOUTH if the user so requires. As the code currently stands, it does handle SOUTH and EAST. I did not add that functionality, so the intention was clearly to support it. However, if you supply SOUTH or EAST, it simply switches off snapping for that direction, which could not possibly be the intention. The only thing I did was fix an existing functionality, I haven't added anything new. If the intent is to stop snapping vertically if you supply SOUTh (or horizontally if you supply EAST), could you please tell me why that was the intention. I understand that Visio and Powerpoint might only support top left snapping, but why does that meand GEF can't supply more functionality. I'm not proposing changing the default at all, just providing more functionality if it is needed.
OK, so you're saying you have something like a right-aligned label, and you want it to snap it's right side to the grid? Or, is your entire diagram right-aligned? You'll have to admit the original description could cause some confusion. You said: "only works for snapping to the top right (NORTH | WEST) corner of a figure." but, top right != NORTHWEST. Anyway, a use case often helps even if the patch is correctly written and preserves existing behavior. There may be alternative approaches, or even additional contributions that would help other GEF clients with the same issues. Thanks.
Created attachment 58780 [details] Bottom Right of Label snapped to grid You're right, I meant top LEFT (NORTH | WEST), not top right. Sorry about that. Yup, I have a figure (say a Label in the Logic example), and I want it to snap it's right side to the grid instead of the left side. I've hacked up the Logic example so that I can give you an example screen shot of what I mean. In the screen shot, you'll see that the bottom right of the label snapped to grid (instead of the default top left). I did this by passing SOUTH | EAST to the snapLocations in snapRectangle. Hopefully the picture can be a little more eloquent than I am. I'm implementing legacy behavior, which means that I don't have the option of saying that the GEF default behavior is the only way that grid snapping works. Thanks for your patience with me on this issue, I really appreciate the feedback and discussion.
Could you submit another version of the patch which doesn't modify the indentation level of the method body?
Created attachment 59006 [details] Patch as requested Here you go Randy. The attached patch.txt doesn't change the indent level.
Wow, that's much more digestible :-). I'm nor keeping up with 3.3 development so I didn't apply the patch, but it seems like the patch would always move the rectangle's left side whenever the position constant includes EAST. This works for the move scenario, but probably breaks the resize-right-edge scenario (only the right side of the rectangle should snap).
The patch does not quite work for resize, to demonstrate: Create a logic diagram and turn on the view + grid. Zoom to 200% Create a Flow Container Move so NORTH WEST corner is on the grid. Now drag resize the SOUTH EAST corner to the grid. The problem is that the NORTH WEST corner should not move on the resize, but there is a noticeable flicker. I think this would be a regression if we put in this patch.
*** Bug 150988 has been marked as a duplicate of this bug. ***
This bug still exists. Has there been any progress?
