Skip to content
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

DrawPathTool: Highlight object that is going to be followed (dots version) #2204

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lpechacek
Copy link
Member

This is a variant of PR#2179 that uses dots for the highlight. The code complexity is higher than with the dashes and the patch would need a bit of tweaking before a merge. The pull request is in place to make the implementation effort public and give users the opportunity to test the change.

@dl3sdo
Copy link
Member

dl3sdo commented Feb 3, 2024

Libor, thank you for the variant. I personally think that the dots should be larger as they are sometimes hard to see.

I encountered some crashes with different error messages:
home/matthias/oom/mapper/src/core/renderables/renderable.cpp:273:33: runtime error: member call on address 0x5601d8d480c0 which does not point to an object of type 'Object'
/home/matthias/oom/mapper/src/core/renderables/renderable.cpp:270:24: runtime error: member call on address 0x5601d9470e10 which does not point to an object of type 'Symbol'
/home/matthias/oom/mapper/src/core/symbols/symbol.h:471:33: runtime error: member access within address 0x5601d9470e10 which does not point to an object of type 'Symbol'
0x5601d9470e10: note: object is of type 'QPinchGesture'

To reproduce the crash:
Start following a line, release the Shift key but then close the line by pressing the Return key.
The highlighting dots appear again when moving the cursor close to the line.
Start again by pressing the Shift Key and pressing the left mouse button twice (and start dragging).

Crash_FollowLine.mp4

@lpechacek
Copy link
Member Author

Thanks, Matthias, for the thorough testing! I've replicated the issue and drafted a fix.

Circumventing finishFollowing() is the key to replicating the crash. It can be done by finishing the line with Enter key while holding the left mouse button. Another possibility is to abort drawing with Escape key while holding LMB again. The bellow patch makes both the code paths call finishFollowing() where we remove the renderables.

Ad the dot size, I agree that they are somewhat small. I also received feedback that they scale with changing magnification but that's something I cannot fix quickly.

What do you thing would be the optimal dot size in terms of an ISOM line symbol size? Width of 505 Footpath, 503 Road, or another size, please?

diff --git a/src/tools/draw_path_tool.cpp b/src/tools/draw_path_tool.cpp
index d1d30ff9429d..09965c9b3d3e 100644
--- a/src/tools/draw_path_tool.cpp
+++ b/src/tools/draw_path_tool.cpp
@@ -925,7 +925,7 @@ void DrawPathTool::finishDrawing()
        }
        
        dragging = false;
-       following = false;
+       finishFollowing();
        setEditingInProgress(false);
        if (!ctrl_pressed)
                angle_helper->setActive(false);
@@ -941,7 +941,7 @@ void DrawPathTool::finishDrawing()
 void DrawPathTool::abortDrawing()
 {
        dragging = false;
-       following = false;
+       finishFollowing();
        setEditingInProgress(false);
        if (!ctrl_pressed)
                angle_helper->setActive(false);

@dl3sdo
Copy link
Member

dl3sdo commented Feb 6, 2024

Thanks, Libor, for the fix which solves the issue for the scenarios described by you and me.

Regarding the dot size I tend to something of at least 503 Road.
I played around and increased the values to

dot_distance = 500;
red_dot->setInnerRadius(150);
white_dot->setInnerRadius(180);

In complex map situations it is unclear which object will be followed
when the user uses the Shift modifier key to follow lines or area object
boundaries. This change makes the object selection more obvious.

As part of the job we are hardening and making more use of
finishFollowing() that removes the highlight renderables for us.

Co-authored-by: Matthias Kuehlewein <[email protected]>
@lpechacek lpechacek force-pushed the draw-path-highlight-followed-object-use-dots branch from 06af0c3 to 9467052 Compare April 3, 2024 09:29
@lpechacek
Copy link
Member Author

Regarding the dot size I tend to something of at least 503 Road.
I played around and increased the values to ...

I've worked with this dot size for a few days. Not bad but not good either. Dots independent of the zoom level would be great.

Screenshot from 2024-04-16 20-37-41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants