-
Notifications
You must be signed in to change notification settings - Fork 445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: snap to grid #1013
Feat: snap to grid #1013
Conversation
Very impressive work (and very well organized PR and commits !) ! I love it. I'll show it to the team asap. |
src/tools/UBGraphicsRuler.cpp
Outdated
@@ -466,6 +466,14 @@ void UBGraphicsRuler::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) | |||
} | |||
else | |||
{ | |||
// snap to grid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be handled in the mouseMoveEvent instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not so easy here, because moving the tools is actually not handled by OpenBoard code, but left to Qt:
OpenBoard/src/tools/UBGraphicsRuler.cpp
Lines 420 to 423 in 1665e2e
if (!mResizing && !mRotating) | |
{ | |
QGraphicsItem::mouseMoveEvent(event); | |
} |
In such a case we would either have to modify much more code and implement the moving by ourselves, or modify the event, what I think is never a good idea. So I did the snapping here (and also for other tools) when releasing the mouse button.
T know that this leads to different behavior for different cases, but personally found that this does not hurt my experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I didn't implement it and didn't know what to expect, I found it very counter-intuitive, and tried three of four times wondering what I was doing wrong with the tools. I only realized it was actually snapping after looking at the code.
This snap feature is quite important imo, and I think it is very important that its behavior is harmonized, for moving as for rotating. I understand that it implies more code rework, but I think it is necessary, to guarantee a good user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This snap feature is quite important imo, and I think it is very important that its behavior is harmonized, for moving as for rotating. I understand that it implies more code rework, but I think it is necessary, to guarantee a good user experience.
Ok, I understand. I think this affects all the C++ tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was much easier than expected and in fact, I can just move the code to mouseMoveEvent
. For the compass, I also improved snapping of the pencil when resizing. It now also rotates the compass, so that you see exactly through which grid point the circle will go.
I have added the changes as a set of fixup commits and will merge them with the original commits before merging this PR. to keep the clean commit structure. Please tell me when you and your team are satisfied with the behavior.
Hi @letsfindaway, did you notice that when snapping an object, there is actually an intermediate position between two lines that is matching, making the moved object snapping at three different positions between two lines, where one would expect the object to only snap to the one of the two lines ? Do you know what is causing this ? Podcast-20240816134730.mp4 |
This is because all four sides of an object are snappy. At one of the position, the bottom side snaps to the grid line, at the next the top side. |
OK. I expect it to be disturbing from a user's perspective, if left as-is. Do you think you could add a visual feedback of what snapped (a bluish or greyish line (or a corner) fade in and out where the object snapped) ? If too complicated, maybe we could detect what part of the selection frame was grabbed (top, left, right, bottom or inside) and react accordingly ? Also, I noticed that if the user moves an object without the selection frame visible, the snap feature is not working. |
I think it could be more intuitive for the user that any object rotation behaves the same way, if Shift is pressed or not. Could you make the old logic (with angleTolerance) be active as long as a rotation is performed on any object, with Shift pressed ? What do you think ? |
When moving, you don't have to grab an object at a side. You can grab it in the middle. So another idea: make it dependent of the direction of the move: moving left snaps left edge etc.
I see, this is only with the pointer tool. With the Play tool it always snaps. Will have a look at this. Edit: see 1847c50 |
Of course line drawing is not rotation, but do I understand you right: You want to have a snap behavior also for rotations or arbitrary objects? But there is already something like this for rotation. It snaps at 30°, 45°, 60°, 90° etc independent of the Shift key. Do you want that this snapping is also only active with Shift? |
No, it's already there, but always active is what I think should change
Yes, exactly. |
I will rebase and merge the fixup commits into the corresponding original feature commits if you're satisfied with the behavior. Edit: Check the behavior of snapping the compass pen when resizing. It will also rotate the compass, but I think this is intuitive. Before it was a pain to get the compass pen on a specific point, because you have to have to rotate and resize independently. |
For this I'm not sure, I'll present the feature to the team tomorrow, discuss this point with them and I'll keep you informed asap. I think that making it dependent of the device move will create a behavior not immediately intuitive (Imagine you want to snap the bottom-left corner of an object with a corner of the grid located in the NE direction, you'll have to go beyond the desired position with the object, and then press Shift and come back in the SW direction). You didn't answer to the idea of adding a visual effect/feedback when something snaps. What do you think of this idea ? I think it's the best because it lets all the possibilities that offers your first implementation, and increase the interactions/feedback with the user, which is a global lack of OpenBoard that need to be improved imo. I imagine it not too hard to implement but maybe it's here that I'm mistaking. I'm currently pushing for more feedback, for depth handling for example (An improvement that I plan to propose to the team in this regard is the addition of a permanent indication of the depth of the object in the selection frame), and I actually like the first behavior you proposed, I just think we need to tell more to the user. |
Ahh, I missed that. I think the visual feedback around an object is problematic when the object is not completely visible on the view. Then the feedback would also not be visible. What about displaying the feedback near the cursor? Edit: Note also that I'm snapping points, not sides. So the indication will indicate the snap point, upper-left, lower-left, upper-right or lower-right.. |
Good point. But as the visual feedback would be very frequent during snapping, I think the user would know what's happening.
Wow ! It could be very cool, and it would resolve the issue you pointed out. I saw that there is animation-related classes in Qt (QPropertyAnimation). So maybe it would be possible to have a blueish (I don't know why I imagine it blue, I may be influenced by diagrams.net ^^) or greyish line or angle appears and rapidly fades out, indicating what side or corner just snapped. Could it be around the cursor instead of near it (at the top-left of the cursor if top-left snap, at cursor's right if right side of the object snapped, etc) ? Edit: but maybe you meant it exactly that by "near" |
2c904c1
to
d135e66
Compare
I pushed an initial implementation of the snap indicator showing fading triangles around the cursor. For a crossed background the triangles indicate the snapped corner. For a ruled background (only horizontal lines) the triangles always point to the upper or lower left corner. The visual appearance may be improved - proposals are welcome. The indications are drawn in the new class Edit: I added a circle to the indicator. For me it visually helps associating the indicator with the cursor and to better express a direction. |
Hi @letsfindaway, First of all they like this feature. The main feedback is that the feature should not be disabled if no grid visible. it feels strange that Shift has no effect if no grid. I suggest that for empty backgrounds, the behavior is the same as with crossed background with its current values (so changing grid size also affects plain backgrounds). Combined with your flexible ruler description, that we plan to merge too, but in a second time, I wonder how it should behave though. If you have any idea, let me know. |
Glad to hear!!
Hmm, I see. So let's see what to do.
That would be very easy to implement. But I see some problems with the flexible ruler description. Which pattern should be the "default" used with a blank background? To keep your idea I wold propose to use a simple crossed background also there, because it is most logical for arranging items.
The idea is as follows:
|
No I meant what to do for the empty background, for example if crossed background is no longer on the list of templates. I did not look much at the code, so maybe it's not an issue because crossed background remains on the code no matter what templates are available ? |
We could define the empty background as a crossed background with invisible lines (alpha = 0). |
Good idea ! |
The team and I also decided that the rotation behavior should be simplified. Pressing Shift while rotating anything (drawings, shapes, apps, tools, etc) on the board will apply a step by step rotation of x° (probably 5° by default). And probably this 5° step will be configurable in the preferences. Combined with your work, it should really improve user experience around objects transformations |
Shall this also apply to drawing a straight line with the line tool? Edit: I mean shall we also snap to multiples of 5° in this case? And do we need the angle indication as we have it when rotating an object in this case? |
Yep, and I'm actually implementing that. I'll push it in a few minutes. I'm also changing the precision to 1 decimal, but I'm not finding a simple way to do so for the protractor. Don't hesitate to improve the next commit. |
Yep |
e7b8bce
to
5c0e106
Compare
Wow Thank you ! I just realized it could sound like if I was asking you to implement the feedback but I was going to do it myself, and just wanted to inform you ! But thank you ! Thankfully I decided to go on the triangle today, so we didn't work both on the same thing ^^. |
I'm going to work on snapping the rotation of a multiselection group. Will not do anything for the tools. But note that my feat: stepwise rotation of items commit includes an update of |
6265e72
to
e734289
Compare
e734289
to
55e90a3
Compare
- snap rotation angle to fixed step when pressing shift while rotating or drawing with compass
- snapping is useful on blank background, too - use the same snap points as for a crossed background - this also takes into account the "intermediate lines" switch - so snapping can be switched between 5mm and 10mm
b9bdff8
to
619227f
Compare
From my POV this PR is now ready for merging. Please let me know if you have still comments, questions and/or requests or if features are still missing or should be changed. |
I say thank you for this very useful feature and your impressive work. One question : More details : I observed this issue for horizontal and vertical line. But the end point fits the crossed grid for diagonal lines. |
I would mentioned another issue with the
The solutions:
|
Thanks for reporting! I looked into it: it is a race condition between "snap to grid" and "snap to angle". I have an idea how to solve that but I need some more time to investigate. Currently this works as follows: When snapping the end point of a line, I consider two things:
For most cases this works well, and you can easily snap to a grid point, but also if your cursor is far enough away from an grid point and close enough to the desired angle it snaps to the angle. Try it out for an angle of e.g. 35°! But for angles of 0°, 45° etc the end point of the "snap-to-angle-line" is always closer than the grid point. Therefore snap-to-grid does not work for those cases. My idea is to disable snap-to-angle if the line resulting from snap-to-grid would also satisfy the snap-to-angle condition, i.e. has an angle which is a multiple of 5°. Let me see how this is possible and how it behaves! |
Yes, you're right, and again thanks for reporting. I can tell you that the flexible ruler description is also planned to be integrated, but most probably not for 1.7.2, as this is now in feature-freeze status, but for the next version. I would recommend not to do any ugly workaround in the meantime but to tell people that this is a known limitation for that version which will be solved in the next. |
Thank you for your answers. One more question: |
Currently: no. Do you have any idea how the user interaction could look like? Does the stylus have a button which could be used for that? Typically if the stylus has at least one button or switch, then this is used to switch to eraser. Other ideas like the introduction of a toggle button e.g. in the stylus toolbar to enable/disable snap mode have the drawback that you cannot e.g. start a line without snapping and then end it with snapping enabled. And: I assume that this scenario is currently not a use case in Geneva, so any changes affecting the visible UI will most probably not be merged. So the only idea I have: if the stylus has a button which is not used to switch to eraser mode, then we could investigate whether this button could behave like the |
Thank you for your detailed answer. Currently, as a workaround, I can imagine to remap the |
With an
The benefits are:
|
... but then you have to move between board and keyboard for that - not very comfortable. Another idea: what about a long press. Start drawing your line, and don't move the pen for a second or so. This switches to snap mode for this line. Then draw the line, and it snaps to grid and angle. When lifting the pen again, the snap mode is disabled. We could first try this for line drawing. But it could also be applied for all the other snap operations. That would however need more considerations. Currently we have a lot of "mouse press" and "mouse move" event handlers for the operations. And you see from the number of my commits in this PR, that for each type of object or tool another handler needed to be adapted. This was not a problem when just considering the shift key. I needed an However also here I would first ask @kaamui whether this is something he and his team could imagine. |
I think a simpler - and probably better in terms of UX - would be to have a "magnet" button somewhere on the board to enable/disable snapping.
No I didn't anticipate this issue, and I agree it's a regression for configurations with an interactive whiteboard.
It's true but a far more acceptable drawback I think. With the button clearly visible, you know before starting your line if it will snap or not, and if you want it to snap or not, you click on it before starting your line. |
And maybe @fadikdawoud can help with the enabled/disabled magnet icons ? 😃 |
Good idea. I'd be glad to help out, but I'll need more details to give you a better design. Fill me in on the details, and I’ll take it from there. |
Our designer gave me the feedback that it would be better understood if the animation of the corner snapping was seen directly where it snaps, and that the use case of the snapping happening outside of the visible area is acceptable. I don't if it is possible, but maybe we could use your work (around the mouse cursor) when not visible, and make it appear in the corner of the bounding box of the move item when visible ? Of course this change sounds more complicated and will be added in 1.7.3 if ever possible. |
The snap feature is a work done by @letsfindaway that gives the user the ability, when moving, resizing or rotating an object, to snap to the grid or not, by holding So the good solution - imo - would be to have a button in the stylus toolbar (the one at the bottom of the board), or maybe right to this toolbar if we want to visually express that this button is not a tool like the other ones, that could be enabled/disabled before performing any transformation on an object of the scene. So it would imply to have two icons (a magnet is the usual representation for this kind of features), one for when it is enabled, one for when it is not. |
I think even this would not be too complicated, and could probably even simplify the code, because I then would move the appearing of the snap indicator to the |
I definitely agree! And I would place the button to the right of the "Virtual Keyboard" button, which is also not a tool. Of course I can take the part of adding the code, and @fadikdawoud designs the icon. The other icon in the stylus palette are here: https://github.com/OpenBoard-org/OpenBoard/tree/dev/resources/images/stylusPalette . You see this are 42x42 px icons with transpatrent background. And I would prefer to have it as SVG, as this is more future proof when we better support HiRES displays. |
If you think you can handle it during this month I can merge the modification in the 1.7.2 |
Thank you! That sounds like a solid approach. I'll start working on the icons right away and share the final versions here when they're ready.
I completely agree with SVG part! Placing the button to the right of the "Virtual Keyboard" seems like the best choice, as it aligns with the UI structure. I'd be happy to take on the task of designing the icon. Thanks for pointing me to the references—this will be really helpful. I'll make sure to follow the specifications and provide the icons as SVG files to ensure scalability and compatibility with Hi-Res displays.
I should be able to complete the designs within this month. Once everything is finalized, I’ll send the files over, and we can proceed from there. Thank you for your support. |
I have now started #1100 and propose to continue the discussion there. |
In version 1.7.2, it's more like a 0°/5°/10°/15°/20°/...90° snap. I wonder if there is a way to get the 0°/45°/90° snap back. |
Yes, it is!! Add the following setting to [App]
RotationAngleStep=45 OpenBoard the still behaves different than before: Without For information about locating this file, see https://github.com/OpenBoard-org/OpenBoard/wiki/Configuration |
This pull request adds a snap-to-grid function when drawing lines and moving tools or objects and resizing objects. In detail it provides the following functions:
Shift
key is no longer used for multi-selection. Instead now only theCtrl
key works for that. Before, both keys were used for multi-selection.Shift
key is now used to activate snap-to-grid. The snap function is only active while this key is pressed. IfShift
is not pressed, all draw, move and resize operations do not snap.Shift
is not pressed. This was requested by several people who wanted to draw "near horizontal" lines.With the
Shift
key pressed, some points snap to the grid as follows:The individual tools and objects behave as follows when the
Shift
key is pressed and the tool or object is moved:The individual tools and objects behave as follows when the
Shift
key is pressed and the tool or object is resized:Shift
is pressed, then the radius snaps so that the circle goes through the closest grid point.When an object is in a rotated state, then the corners of the bounding rectangle of the rotated object define the points to snap.
For a Line, the snap points are not determined by the outline of the line, but by the actual line start and end points inside the outline. This works however only if the line is moved or resized in the same session where it was created. After a document was serialized and loaded again, the line property is lost and the line is handled like any other arbitrary drawing.
The implementation of the snap function is as follows:
UBGraphicsScene
, functions are added to compute the snap vector, i.e. the amount by which we should move an object so that it snaps.UBBoardView
to theUBGraphicsScene
and the snap functions are used.UBBoardView
.UBGraphicsDelegateFrame
.When at some later time the flexible background definitions are implemented (PR #888), then only the single
UBGraphicsScene::snap()
function must be adapted. All the other logic is not affected.This PR is related to the following issues: