First snapping implementation #507

mbfraga · 2021-03-05T05:34:47Z

Implements snapping between objects. Lays the groundwork for a future, more complete implementation.

Notable features:

Stateless implementation for snapping.
Snapping on object move
First pass cosmetic implementation for guide lines and snap points (probably needs some reworking)

Notable missing features are:

Adding the snapping functionality to other transformations (resize). It should mostly require some hooking up of existing code
Smarter generation of candidates, possibly only snapping to artboard items if the item is within an artboard. The code for this is mostly there, we mainly need a method to generate the list of items from an artboard.

Alecaddd

Thank you so much for this amazing submission.
Pretty much every comment I left is purely from a code formatting point of view. Let me know if you don't want to take care of this I can do that for you :D

Alecaddd · 2021-03-06T04:59:43Z

src/Lib/Canvas.vala

+        // This is a temporary approach to end operations. In the future we may want to have more specific
+        // methods.
+        selected_bound_manager.alert_held_button_release ();


Maybe this method could be called on_butten_release ()

Alecaddd · 2021-03-06T05:01:43Z

src/Lib/Managers/SelectedBoundManager.vala

+        Lib.Items.CanvasItem item,
+        double event_x,
+        double event_y
+        ) {


We can inline this with private void move_from_event (Items.CanvasItem item, double event_x, double event_y) {

Alecaddd · 2021-03-06T05:01:56Z

src/Lib/Managers/SelectedBoundManager.vala

+        double event_x,
+        double event_y
+        ) {
+


Remove the empty line.

Alecaddd · 2021-03-06T05:03:40Z

src/Lib/Managers/SelectedBoundManager.vala

+        // Make adjustment basted on snaps
+        // Double the sensitivity to allow for reuse of grid after snap


We usually try to end all the inline comments with a period.

Alecaddd · 2021-03-06T05:05:04Z

src/Lib/Managers/SelectedBoundManager.vala

-        // then be translated again.
+        // Make adjustment basted on snaps
+        // Double the sensitivity to allow for reuse of grid after snap
+        var sensitivity = (int) Utils.Snapping.adjusted_sensitivity (canvas.current_scale);


If we always need an INTEGER from this method, we should return directly an integer from the method instead of type casting it every time we use it.

Alecaddd · 2021-03-06T05:40:54Z

src/Utils/Snapping.vala

+    List<weak Goo.CanvasItem> v_candidates,
+    List<weak Goo.CanvasItem> h_candidates,
+    List<Lib.Items.CanvasItem> selection


Indent these attributes.

Alecaddd · 2021-03-06T05:41:35Z

src/Utils/Snapping.vala

+          var candidate_item = cand as Lib.Items.CanvasItem;
+          if (candidate_item != null && selection.find (candidate_item) == null) {
+            populate_vertical_snaps (candidate_item, ref grid.v_snaps);
+          }


The indentation here is a bit wrong, as well as in the next foreach loop.

Alecaddd · 2021-03-06T05:41:58Z

src/Utils/Snapping.vala

+        return grid;
+    }
+
+    /// Populates the horizontal snaps of an item


Comment block for methods, please :D

Alecaddd · 2021-03-06T05:42:38Z

src/Utils/Snapping.vala

+
+    /// Populates the horizontal snaps of an item
+    private static void populate_horizontal_snaps (Lib.Items.CanvasItem item, ref Gee.HashMap<int, SnapMeta> map) {
+        int x_1 = (int)item.bounds.x1;


Space after (int)

Alecaddd · 2021-03-06T05:43:51Z

src/Utils/Snapping.vala

+        if (map.has_key (pos)) {
+            SnapMeta k = map.get (pos);
+            k.normals.add (n1);
+            k.normals.add (n2);
+            k.normals.add (n3);
+            k.polarity += polarity;
+        }
+        else {
+            var v = new SnapMeta ();
+            v.normals = new Gee.HashSet<int> ();
+            v.normals.add (n1);
+            v.normals.add (n2);
+            v.normals.add (n3);
+            v.polarity = polarity;
+            map.set (pos, v);
+        }


Since nothing happens after the callback condition, we can do an early return; in the first condition, and avoid the else.

mbfraga and others added 6 commits March 4, 2021 21:51

Add Snaping Util and SnapManager

372c22a

Linter pass

3a9ad16

Add some documentation

55900c4

Merge remote-tracking branch 'akiraux/master' into wip/mbfraga/snapping3

ce1e037

Merge branch 'master' into wip/mbfraga/snapping3

936609c

Fix ordering bug

211f69b

Alecaddd requested changes Mar 6, 2021

View reviewed changes

Alecaddd mentioned this pull request Mar 6, 2021

Beta Release Wishlist #358

Open

46 tasks

Address review comments

e2abec0

mbfraga requested a review from Alecaddd March 6, 2021 16:55

Alecaddd mentioned this pull request Mar 6, 2021

WIP: Object Snapping #502

Closed

3 tasks

Fix build regression

3dff25e

Alecaddd merged commit 3dff25e into akiraux:master Mar 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First snapping implementation #507

First snapping implementation #507

mbfraga commented Mar 5, 2021

Alecaddd left a comment

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

Alecaddd Mar 6, 2021

		// Make adjustment basted on snaps
		// Double the sensitivity to allow for reuse of grid after snap

First snapping implementation #507

First snapping implementation #507

Conversation

mbfraga commented Mar 5, 2021

Alecaddd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment