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

[3D] Fix some issues in 3D point cloud editing tools #60917

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Withalion
Copy link
Contributor

@Withalion Withalion commented Mar 10, 2025

Description

This PR fixes some new issues found while using point cloud editing tools.

  • Above & below line tools don't crash anymore when user specifies only one point and right clicks
  • Editing tools get disabled when editing is also disabled (control returns to camera)
  • Add minimum size for paintbrush tool, so users don't scroll to doom and back with no effect
  • Remove movement "mode" in polygonal selection tool when already some area is selected
  • Change movement in paintbrush tool, which is now possible when user holds SPACE
  • Fix build error, which was introduced as a fix for QGIS crash when closing 3D window with editing tools open (more info in commit message)
  • Editing toolbar keeps open/closed throughout sessions

Fixes msvc build issue caused by uniqueqobject pointer, we revert back
to raw pointers. Fixes QGIS crash on 3D window closed when editing tools
are selected. We can't delete QgsRubberBand3D entities from this class
as they are owned by RubberBandRootEntity, which deletes them on windows
close. Only point rubberbands leave artifacts in scene whene deleted by
default destructor.
@github-actions github-actions bot added this to the 3.44.0 milestone Mar 10, 2025
Copy link

github-actions bot commented Mar 10, 2025

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 9303315)

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 9303315)

Comment on lines +172 to +173
if ( mScreenPoints.size() != 2 )
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not support above/below lines with more than 2 points?
I'd move the

  if ( mScreenPoints.size() < 3 )
    return;

guard at the top of the run method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not support above/below lines with more than 2 points?

After discussion with @wonder-sk we decided that we will support just 2 points, not more.

I'd move the

if ( mScreenPoints.size() < 3 )
   return;

guard at the top of the run method.

The thing is we add the screen edges points when using line tool right before that guard

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