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.

Sign up for GitHub

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 7 commits into
base: master
Choose a base branch
from
+71 −36
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
93c80de
Fix line editing tool crash when used wrong
Withalion Mar 7, 2025
7fc4600
Disable editing tool, when disabling editing in 3D
Withalion Mar 7, 2025
153bb02
Add minimum size for paintbrush 3D editing tool
Withalion Mar 7, 2025
edae588
Fix paintbrush editing tool movement
Withalion Mar 7, 2025
35b38da
Fix build issue & crash on closing 3D window
Withalion Mar 7, 2025
9303315
Keep 3D editing toolbar position on closing
Withalion Mar 10, 2025
4049525
Finish editing when 2. point selected with line tool
Withalion Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/3d/qgs3dmapcanvas.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ bool Qgs3DMapCanvas::eventFilter( QObject *watched, QEvent *event )
case QEvent::KeyPress:
mMapTool->keyPressEvent( static_cast<QKeyEvent *>( event ) );
break;
case QEvent::KeyRelease:
mMapTool->keyReleaseEvent( static_cast<QKeyEvent *>( event ) );
break;
case QEvent::Wheel:
mMapTool->mouseWheelEvent( static_cast<QWheelEvent *>( event ) );
break;
Expand Down
5 changes: 5 additions & 0 deletions src/3d/qgs3dmaptool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ void Qgs3DMapTool::keyPressEvent( QKeyEvent *event )
Q_UNUSED( event )
}

void Qgs3DMapTool::keyReleaseEvent( QKeyEvent *event )
{
Q_UNUSED( event )
}

void Qgs3DMapTool::mouseWheelEvent( QWheelEvent *event )
{
Q_UNUSED( event )
Expand Down
2 changes: 2 additions & 0 deletions src/3d/qgs3dmaptool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class _3D_EXPORT Qgs3DMapTool : public QObject
virtual void mouseMoveEvent( QMouseEvent *event );
//! Reimplement to handle key press \a event forwarded by the parent Qgs3DMapCanvas
virtual void keyPressEvent( QKeyEvent *event );
//! Reimplement to handle key release \a event forwarded by the parent Qgs3DMapCanvas
virtual void keyReleaseEvent( QKeyEvent *event );
//! Reimplement to handle mouse wheel \a event forwarded by the parent Qgs3DMapCanvas
virtual void mouseWheelEvent( QWheelEvent *event );

Expand Down
11 changes: 2 additions & 9 deletions src/3d/qgsrubberband3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,8 @@ void QgsRubberBand3D::setupPolygon( Qt3DCore::QEntity *parentEntity )
mPolygonEntity->addComponent( mPolygonMaterial );
}

QgsRubberBand3D::~QgsRubberBand3D()
{
if ( mPolygonEntity )
mPolygonEntity->deleteLater();
if ( mLineEntity )
mLineEntity->deleteLater();
if ( mMarkerEntity )
mMarkerEntity->deleteLater();
}
QgsRubberBand3D::~QgsRubberBand3D() = default;


float QgsRubberBand3D::width() const
{
Expand Down
52 changes: 31 additions & 21 deletions src/app/3d/qgs3dmapcanvaswidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,16 @@ Qgs3DMapCanvasWidget::Qgs3DMapCanvasWidget( const QString &name, bool isDocked )
// Editing toolbar
mEditingToolBar = new QToolBar( this );
mEditingToolBar->setWindowTitle( tr( "Editing Toolbar" ) );
mEditingToolBar->setVisible( false );
mEditingToolsMenu = new QMenu( this );

mPointCloudEditingToolbar = new QToolBar( this );

mActionToggleEditing = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionToggleEditing.svg" ) ), tr( "Toggle editing" ), this );
mActionToggleEditing->setCheckable( true );
connect( mActionToggleEditing, &QAction::triggered, this, [] { QgisApp::instance()->toggleEditing( QgisApp::instance()->activeLayer() ); } );
connect( mActionToggleEditing, &QAction::triggered, this, [this] {
QgisApp::instance()->toggleEditing( QgisApp::instance()->activeLayer() );
mCanvas->setMapTool( nullptr );
} );
mActionUndo = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionUndo.svg" ) ), tr( "Undo" ), this );
mActionRedo = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionRedo.svg" ) ), tr( "Redo" ), this );

Expand Down Expand Up @@ -120,8 +122,12 @@ Qgs3DMapCanvasWidget::Qgs3DMapCanvasWidget( const QString &name, bool isDocked )
mClassValidator = new ClassValidator( this );
mCboChangeAttributeValueAction = mPointCloudEditingToolbar->addWidget( mCboChangeAttributeValue );

QAction *actionEditingToolbar = toolBar->addAction( QIcon( QgsApplication::iconPath( "mIconPointCloudLayer.svg" ) ), tr( "Show Editing Toolbar" ), this, [this] { mEditingToolBar->setVisible( !mEditingToolBar->isVisible() ); } );
QAction *actionEditingToolbar = toolBar->addAction( QIcon( QgsApplication::iconPath( "mIconPointCloudLayer.svg" ) ), tr( "Show Editing Toolbar" ) );
actionEditingToolbar->setCheckable( true );
actionEditingToolbar->setChecked(
setting.value( QStringLiteral( "/3D/editingToolbar/visibility" ), false, QgsSettings::Gui ).toBool()
);
connect( actionEditingToolbar, &QAction::toggled, this, &Qgs3DMapCanvasWidget::toggleEditingToolbar );
connect( mCboChangeAttribute, qOverload<int>( &QComboBox::currentIndexChanged ), this, [this]( int ) { onPointCloudChangeAttributeSettingsChanged(); } );
connect( mCboChangeAttributeValue, qOverload<const QString &>( &QComboBox::currentTextChanged ), this, [this]( const QString &text ) {
double newValue = 0;
Expand Down Expand Up @@ -306,7 +312,7 @@ Qgs3DMapCanvasWidget::Qgs3DMapCanvasWidget( const QString &name, bool isDocked )

mMapToolMeasureLine = new Qgs3DMapToolMeasureLine( mCanvas );

mMapToolChangeAttribute.reset( new Qgs3DMapToolPointCloudChangeAttribute( mCanvas ) );
mMapToolChangeAttribute = new Qgs3DMapToolPointCloudChangeAttribute( mCanvas );

mLabelPendingJobs = new QLabel( this );
mProgressPendingJobs = new QProgressBar( this );
Expand Down Expand Up @@ -392,6 +398,7 @@ Qgs3DMapCanvasWidget::Qgs3DMapCanvasWidget( const QString &name, bool isDocked )
} );

updateLayerRelatedActions( QgisApp::instance()->activeLayer() );
mEditingToolBar->setVisible( setting.value( QStringLiteral( "/3D/editingToolbar/visibility" ), false, QgsSettings::Gui ).toBool() );

QList<QAction *> toolbarMenuActions;
// Set action names so that they can be used in customization
Expand All @@ -415,11 +422,6 @@ Qgs3DMapCanvasWidget::Qgs3DMapCanvasWidget( const QString &name, bool isDocked )
Qgs3DMapCanvasWidget::~Qgs3DMapCanvasWidget()
{
delete mDockableWidgetHelper;
// we don't want canvas to reset the map tool as it is managed by unique_ptr
if ( mMapToolChangeAttribute.get() == mCanvas->mapTool() )
{
mCanvas->setMapTool( nullptr );
}
}

void Qgs3DMapCanvasWidget::saveAsImage()
Expand Down Expand Up @@ -481,10 +483,11 @@ void Qgs3DMapCanvasWidget::changePointCloudAttributeByPaintbrush()
if ( !action )
return;

mCanvas->setMapTool( nullptr );
mMapToolChangeAttribute.reset( new Qgs3DMapToolPointCloudChangeAttributePaintbrush( mCanvas ) );
mCanvas->requestActivate();
mMapToolChangeAttribute->deleteLater();
mMapToolChangeAttribute = new Qgs3DMapToolPointCloudChangeAttributePaintbrush( mCanvas );
onPointCloudChangeAttributeSettingsChanged();
mCanvas->setMapTool( mMapToolChangeAttribute.get() );
mCanvas->setMapTool( mMapToolChangeAttribute );
mEditingToolsAction->setIcon( action->icon() );
}

Expand All @@ -494,10 +497,10 @@ void Qgs3DMapCanvasWidget::changePointCloudAttributeByPolygon()
if ( !action )
return;

mCanvas->setMapTool( nullptr );
mMapToolChangeAttribute.reset( new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::Polygon ) );
mMapToolChangeAttribute->deleteLater();
mMapToolChangeAttribute = new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::Polygon );
onPointCloudChangeAttributeSettingsChanged();
mCanvas->setMapTool( mMapToolChangeAttribute.get() );
mCanvas->setMapTool( mMapToolChangeAttribute );
mEditingToolsAction->setIcon( action->icon() );
}

Expand All @@ -507,10 +510,10 @@ void Qgs3DMapCanvasWidget::changePointCloudAttributeByAboveLine()
if ( !action )
return;

mCanvas->setMapTool( nullptr );
mMapToolChangeAttribute.reset( new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::AboveLine ) );
mMapToolChangeAttribute->deleteLater();
mMapToolChangeAttribute = new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::AboveLine );
onPointCloudChangeAttributeSettingsChanged();
mCanvas->setMapTool( mMapToolChangeAttribute.get() );
mCanvas->setMapTool( mMapToolChangeAttribute );
mEditingToolsAction->setIcon( action->icon() );
}

Expand All @@ -520,10 +523,10 @@ void Qgs3DMapCanvasWidget::changePointCloudAttributeByBelowLine()
if ( !action )
return;

mCanvas->setMapTool( nullptr );
mMapToolChangeAttribute.reset( new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::BelowLine ) );
mMapToolChangeAttribute->deleteLater();
mMapToolChangeAttribute = new Qgs3DMapToolPointCloudChangeAttributePolygon( mCanvas, Qgs3DMapToolPointCloudChangeAttributePolygon::BelowLine );
onPointCloudChangeAttributeSettingsChanged();
mCanvas->setMapTool( mMapToolChangeAttribute.get() );
mCanvas->setMapTool( mMapToolChangeAttribute );
mEditingToolsAction->setIcon( action->icon() );
}

Expand Down Expand Up @@ -612,6 +615,13 @@ void Qgs3DMapCanvasWidget::toggleNavigationWidget( const bool visibility )
setting.setValue( QStringLiteral( "/3D/navigationWidget/visibility" ), visibility, QgsSettings::Gui );
}

void Qgs3DMapCanvasWidget::toggleEditingToolbar( const bool visibility )
{
mEditingToolBar->setVisible( visibility );
QgsSettings setting;
setting.setValue( QStringLiteral( "/3D/editingToolbar/visibility" ), visibility, QgsSettings::Gui );
}

void Qgs3DMapCanvasWidget::toggleFpsCounter( const bool visibility )
{
mLabelFpsCounter->setVisible( visibility );
Expand Down
3 changes: 2 additions & 1 deletion src/app/3d/qgs3dmapcanvaswidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class APP_EXPORT Qgs3DMapCanvasWidget : public QWidget
void changePointCloudAttributeByBelowLine();
void exportScene();
void toggleNavigationWidget( bool visibility );
void toggleEditingToolbar( bool visibility );
void toggleFpsCounter( bool visibility );
void toggleDebugWidget( bool visibility ) const;
void toggleDebugWidget() const;
Expand Down Expand Up @@ -143,7 +144,7 @@ class APP_EXPORT Qgs3DMapCanvasWidget : public QWidget
QTimer *mLabelNavSpeedHideTimeout = nullptr;
Qgs3DMapToolIdentify *mMapToolIdentify = nullptr;
Qgs3DMapToolMeasureLine *mMapToolMeasureLine = nullptr;
QObjectUniquePtr<Qgs3DMapToolPointCloudChangeAttribute> mMapToolChangeAttribute;
Qgs3DMapToolPointCloudChangeAttribute *mMapToolChangeAttribute = nullptr;
std::unique_ptr<QgsMapToolExtent> mMapToolExtent;
QgsMapTool *mMapToolPrevious = nullptr;
QMenu *mExportMenu = nullptr;
Expand Down
23 changes: 18 additions & 5 deletions src/app/3d/qgs3dmaptoolpointcloudchangeattributepaintbrush.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ void Qgs3DMapToolPointCloudChangeAttributePaintbrush::activate()
void Qgs3DMapToolPointCloudChangeAttributePaintbrush::deactivate()
{
restart();
// this makes sure there are no leftover artifacts when switching from paintbrush tool to another
mSelectionRubberBand->setMarkersEnabled( false );
mSelectionRubberBand.reset();
mIsActive = false;
mCanvas->cameraController()->setInputHandlersEnabled( true );
Expand Down Expand Up @@ -143,9 +145,12 @@ void Qgs3DMapToolPointCloudChangeAttributePaintbrush::mouseWheelEvent( QWheelEve
const QgsSettings settings;
const bool reverseZoom = settings.value( QStringLiteral( "qgis/reverse_wheel_zoom" ), false ).toBool();
const bool shrink = reverseZoom ? event->angleDelta().y() < 0 : event->angleDelta().y() > 0;
if ( shrink && mSelectionRubberBand->width() <= 5 )
return;
// "Normal" mouse have an angle delta of 120, precision mouses provide data faster, in smaller steps
const double zoomFactor = ( shrink ? 0.8 : 1.25 ) / 120.0 * std::fabs( event->angleDelta().y() );
mSelectionRubberBand->setWidth( mSelectionRubberBand->width() * zoomFactor );
const float newWidth = mSelectionRubberBand->width() * zoomFactor < 5 ? 5 : mSelectionRubberBand->width() * static_cast<float>( zoomFactor );
mSelectionRubberBand->setWidth( newWidth );
}

void Qgs3DMapToolPointCloudChangeAttributePaintbrush::keyPressEvent( QKeyEvent *event )
Expand All @@ -155,10 +160,18 @@ void Qgs3DMapToolPointCloudChangeAttributePaintbrush::keyPressEvent( QKeyEvent *
restart();
}

if ( !mIsClicked && event->key() == Qt::Key_Space )
if ( event->key() == Qt::Key_Space && !event->isAutoRepeat() )
{
mCanvas->cameraController()->setInputHandlersEnabled( true );
mIsMoving = true;
}
}

void Qgs3DMapToolPointCloudChangeAttributePaintbrush::keyReleaseEvent( QKeyEvent *event )
{
if ( event->key() == Qt::Key_Space && !event->isAutoRepeat() )
{
const bool newState = !mCanvas->cameraController()->hasInputHandlersEnabled();
mCanvas->cameraController()->setInputHandlersEnabled( newState );
mIsMoving = newState;
mCanvas->cameraController()->setInputHandlersEnabled( false );
mIsMoving = false;
}
}
1 change: 1 addition & 0 deletions src/app/3d/qgs3dmaptoolpointcloudchangeattributepaintbrush.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class Qgs3DMapToolPointCloudChangeAttributePaintbrush : public Qgs3DMapToolPoint
void mouseMoveEvent( QMouseEvent *event ) override;
void mouseWheelEvent( QWheelEvent *event ) override;
void keyPressEvent( QKeyEvent *event ) override;
void keyReleaseEvent( QKeyEvent *event ) override;

private:
void run() override;
Expand Down
7 changes: 7 additions & 0 deletions src/app/3d/qgs3dmaptoolpointcloudchangeattributepolygon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ void Qgs3DMapToolPointCloudChangeAttributePolygon::mouseReleaseEvent( QMouseEven
mPolygonRubberBand->addPoint( screenEdgePoint );
mPolygonRubberBand->addPoint( newPoint );
}
else if ( mScreenPoints.size() == 2 )
{
run();
restart();
}
}
}
else if ( event->button() == Qt::RightButton )
Expand Down Expand Up @@ -169,6 +174,8 @@ void Qgs3DMapToolPointCloudChangeAttributePolygon::run()

if ( mToolType == AboveLine || mToolType == BelowLine )
{
if ( mScreenPoints.size() != 2 )
return;
Comment on lines +177 to +178
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

const double y = mToolType == AboveLine ? 0 : mCanvas->height();
mScreenPoints.append( { QgsPointXY( mScreenPoints[1].x(), y ), QgsPointXY( mScreenPoints[0].x(), y ) } );
}
Expand Down
Loading