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

Change how "Get Z value from project terrain" tool is presented in Mesh Editing #60709

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Sets the expressions for the coordinates transformation.
Expressions are optional for each coordinate, the coordinate will not be transformed if the string is void.
%End

bool calculate( QgsMeshLayer *layer );
bool calculate( QgsMeshLayer *layer, QgsProject *project = 0 );
%Docstring
Calculates the transformed vertices of the mesh ``layer``, returns ``False`` if this leads to topological or geometrical errors.
The mesh layer must be in edit mode.
Expand All @@ -146,13 +146,26 @@ The mesh layer must be in edit mode.

this method not apply new vertices to the mesh layer but only store the calculated transformation
that can be apply later with :py:func:`QgsMeshEditor.advancedEdit()`
Comment on lines 147 to 148
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is public API, but anyway, could you update these sentences while at it, please? Thanks.

   This method does not apply new vertices to the mesh layer but only stores the calculated transformation
   that can be applied later with :py:func:`QgsMeshEditor.advancedEdit()`


:param layer:

.. versionadded:: 3.44
%End

QgsMeshVertex transformedVertex( QgsMeshLayer *layer, int vertexIndex ) const;
%Docstring
Returns the transformed vertex from its index ``vertexIndex`` for the mesh ``layer``

If ``layer`` is not the same than the one used to make the calculation, this will create an undefined behavior
%End

void setZFromTerrain( bool enable );
%Docstring
Sets if Z values for vertices should be obtained from project terrain, instead of expression.

:param enable:

.. versionadded:: 3.44
%End

};
Expand Down
15 changes: 14 additions & 1 deletion python/core/auto_generated/mesh/qgsmeshadvancedediting.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Sets the expressions for the coordinates transformation.
Expressions are optional for each coordinate, the coordinate will not be transformed if the string is void.
%End

bool calculate( QgsMeshLayer *layer );
bool calculate( QgsMeshLayer *layer, QgsProject *project = 0 );
%Docstring
Calculates the transformed vertices of the mesh ``layer``, returns ``False`` if this leads to topological or geometrical errors.
The mesh layer must be in edit mode.
Expand All @@ -146,13 +146,26 @@ The mesh layer must be in edit mode.

this method not apply new vertices to the mesh layer but only store the calculated transformation
that can be apply later with :py:func:`QgsMeshEditor.advancedEdit()`

:param layer:

.. versionadded:: 3.44
%End

QgsMeshVertex transformedVertex( QgsMeshLayer *layer, int vertexIndex ) const;
%Docstring
Returns the transformed vertex from its index ``vertexIndex`` for the mesh ``layer``

If ``layer`` is not the same than the one used to make the calculation, this will create an undefined behavior
%End

void setZFromTerrain( bool enable );
%Docstring
Sets if Z values for vertices should be obtained from project terrain, instead of expression.

:param enable:

.. versionadded:: 3.44
%End

};
Expand Down
83 changes: 27 additions & 56 deletions src/app/mesh/qgsmeshtransformcoordinatesdockwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,16 @@ QgsMeshTransformCoordinatesDockWidget::QgsMeshTransformCoordinatesDockWidget( QW
connect( mButtonPreview, &QToolButton::clicked, this, &QgsMeshTransformCoordinatesDockWidget::calculate );
connect( mButtonApply, &QPushButton::clicked, this, &QgsMeshTransformCoordinatesDockWidget::apply );
connect( mButtonImport, &QToolButton::toggled, this, &QgsMeshTransformCoordinatesDockWidget::onImportVertexClicked );
connect( mGetZValuesButton, &QPushButton::clicked, this, &QgsMeshTransformCoordinatesDockWidget::updateZValuesFromTerrain );

connect( mCheckBoxZ, &QCheckBox::toggled, this, [=]( const bool checked ) {
if ( checked )
mCheckBoxZFromProjectTerrain->setChecked( !checked );
} );
connect( mCheckBoxZFromProjectTerrain, &QCheckBox::toggled, this, [=]( const bool checked ) {
if ( checked )
mCheckBoxZ->setChecked( !checked );
} );
connect( mCheckBoxZFromProjectTerrain, &QCheckBox::toggled, this, &QgsMeshTransformCoordinatesDockWidget::updateButton );
}

QgsExpressionContext QgsMeshTransformCoordinatesDockWidget::createExpressionContext() const
Expand Down Expand Up @@ -104,8 +113,6 @@ void QgsMeshTransformCoordinatesDockWidget::setInput( QgsMeshLayer *layer, const
}
}

mGetZValuesButton->setDisabled( vertexIndexes.empty() );

importVertexCoordinates();
updateButton();
emit calculationUpdated();
Expand All @@ -120,10 +127,13 @@ void QgsMeshTransformCoordinatesDockWidget::calculate()
mTransformVertices.clear();
mTransformVertices.setInputVertices( mInputVertices );
mTransformVertices.setExpressions( mCheckBoxX->isChecked() ? mExpressionEditX->expression() : QString(), mCheckBoxY->isChecked() ? mExpressionEditY->expression() : QString(), mCheckBoxZ->isChecked() ? mExpressionEditZ->expression() : QString() );
QgsExpressionContext context;
context.appendScope( QgsExpressionContextUtils::projectScope( QgsProject::instance() ) );

mIsResultValid = mTransformVertices.calculate( mInputLayer );
if ( mCheckBoxZFromProjectTerrain->isChecked() )
{
mTransformVertices.setZFromTerrain( mCheckBoxZFromProjectTerrain->isChecked() );
}

mIsResultValid = mTransformVertices.calculate( mInputLayer, QgsProject::instance() );

mIsCalculated = true;
mButtonApply->setEnabled( mIsResultValid );
Expand All @@ -133,6 +143,7 @@ void QgsMeshTransformCoordinatesDockWidget::calculate()

void QgsMeshTransformCoordinatesDockWidget::updateButton()
{
bool modifyXYZSelected = false;
mButtonApply->setEnabled( false );
bool isCalculable = mInputLayer && !mInputVertices.isEmpty();
if ( isCalculable )
Expand All @@ -141,6 +152,11 @@ void QgsMeshTransformCoordinatesDockWidget::updateButton()
for ( const QCheckBox *cb : std::as_const( mCheckBoxes ) )
isCalculable |= cb->isChecked();

if ( isCalculable )
{
modifyXYZSelected = true;
}

if ( isCalculable )
{
for ( int i = 0; i < mCheckBoxes.count(); ++i )
Expand All @@ -149,6 +165,11 @@ void QgsMeshTransformCoordinatesDockWidget::updateButton()
isCalculable &= !checked || mExpressionLineEdits.at( i )->isValidExpression();
}
}

if ( !modifyXYZSelected && mCheckBoxZFromProjectTerrain->isChecked() )
{
isCalculable = true;
}
}

mButtonPreview->setEnabled( isCalculable );
Expand All @@ -163,56 +184,6 @@ void QgsMeshTransformCoordinatesDockWidget::apply()
emit applied();
}

void QgsMeshTransformCoordinatesDockWidget::updateZValuesFromTerrain()
{
if ( mInputVertices.empty() )
return;

QList<int> modifiedVerticesIndexes;
QList<double> newZValues;

const QgsAbstractTerrainProvider *terrainProvider = QgsProject::instance()->elevationProperties()->terrainProvider();

if ( terrainProvider == nullptr )
return;

const QgsCoordinateTransform transformation = QgsCoordinateTransform( mInputLayer->crs(), terrainProvider->crs(), QgsProject::instance() );

QgsPointXY point;
bool vertexTransformed;
double elevation;

for ( int i = 0; i < mInputVertices.count(); i++ )
{
const int vertexIndex = mInputVertices.at( i );
const QgsPoint vertex = mInputLayer->nativeMesh()->vertex( vertexIndex );

try
{
point = transformation.transform( vertex.x(), vertex.y() );
vertexTransformed = true;
}
catch ( const QgsCsException & )
{
vertexTransformed = false;
}

if ( vertexTransformed )
{
elevation = terrainProvider->heightAt( point.x(), point.y() );
if ( !std::isnan( elevation ) )
{
modifiedVerticesIndexes.push_back( vertexIndex );
newZValues.push_back( elevation );
}
}
}

emit aboutToBeApplied();
mInputLayer->meshEditor()->changeZValues( modifiedVerticesIndexes, newZValues );
emit applied();
}

void QgsMeshTransformCoordinatesDockWidget::onImportVertexClicked( bool checked )
{
if ( checked )
Expand Down
7 changes: 0 additions & 7 deletions src/app/mesh/qgsmeshtransformcoordinatesdockwidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ class APP_EXPORT QgsMeshTransformCoordinatesDockWidget : public QgsDockWidget, p
//! Returns whether the calculation has been done
bool isCalculated() const;

/**
* Updates Z values of selected vertices from QGIS project terrain provider
*
* \since QGIS 3.42
*/
void updateZValuesFromTerrain();

signals:
//! Emitted when the calculation of the transform is done
void calculationUpdated();
Expand Down
62 changes: 58 additions & 4 deletions src/core/mesh/qgsmeshadvancedediting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
#include "qgsmeshlayer.h"
#include "qgsexpression.h"
#include "qgsexpressioncontextutils.h"

#include "qgsproject.h"
#include "qgsprojectelevationproperties.h"
#include "qgsterrainprovider.h"

QgsMeshAdvancedEditing::QgsMeshAdvancedEditing() = default;

Expand Down Expand Up @@ -610,7 +612,7 @@ QString QgsMeshEditRefineFaces::text() const
return QObject::tr( "Refine %n face(s)", nullptr, mInputFaces.count() );
}

bool QgsMeshTransformVerticesByExpression::calculate( QgsMeshLayer *layer )
bool QgsMeshTransformVerticesByExpression::calculate( QgsMeshLayer *layer, QgsProject *project )
{
if ( !layer || !layer->meshEditor() || !layer->nativeMesh() )
return false;
Expand Down Expand Up @@ -657,14 +659,29 @@ bool QgsMeshTransformVerticesByExpression::calculate( QgsMeshLayer *layer )
}

QgsExpression expressionZ;
if ( calcZ )
if ( calcZ || mZFromTerrain )
{
expressionZ = QgsExpression( mExpressionZ );
expressionZ.prepare( &context );
mNewZValues.reserve( inputCount );
mOldZValues.reserve( inputCount );
}

QgsCoordinateTransform transformation;
const QgsAbstractTerrainProvider *terrainProvider = nullptr;

if ( mZFromTerrain )
{
if ( project )
{
terrainProvider = project->elevationProperties()->terrainProvider();
if ( terrainProvider )
{
transformation = QgsCoordinateTransform( layer->crs(), terrainProvider->crs(), project );
}
}
}

for ( int i = 0; i < mInputVertices.count(); ++i )
{
const int vertexIndex = mInputVertices.at( i );
Expand Down Expand Up @@ -721,7 +738,7 @@ bool QgsMeshTransformVerticesByExpression::calculate( QgsMeshLayer *layer )
return false;
}

if ( calcZ )
if ( calcZ && !mZFromTerrain )
{
double z = std::numeric_limits<double>::quiet_NaN();
if ( zvar.isValid() )
Expand All @@ -734,6 +751,38 @@ bool QgsMeshTransformVerticesByExpression::calculate( QgsMeshLayer *layer )
mNewZValues.append( z );
mOldZValues.append( vert.z() );
}

if ( mZFromTerrain )
{
if ( terrainProvider )
{
Comment on lines +753 to +756
Copy link
Contributor

Choose a reason for hiding this comment

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

Group those two?

    if ( mZFromTerrain && terrainProvider )
    {

QgsPointXY point;
bool vertexTransformed;
double elevation;

try
{
point = transformation.transform( vert.x(), vert.y() );
Copy link
Contributor

Choose a reason for hiding this comment

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

Harissou has a point, if calcX || calcY we should be transforming mNewXYValues.constLast() instead

vertexTransformed = true;
}
catch ( const QgsCsException & )
{
vertexTransformed = false;
}

if ( vertexTransformed )
{
Comment on lines +771 to +772
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, I wonder if it is safe to skip vertices that failed the transform, or if mNewZValues needs to be matching mNewXYValues in size and we should just return false on exception.

elevation = terrainProvider->heightAt( point.x(), point.y() );
// if elevation at terrain provider is NaN, use the original vertex Z value
if ( std::isnan( elevation ) )
{
elevation = vert.z();
}
mNewZValues.append( elevation );
mOldZValues.append( vert.z() );
}
}
}
}

auto transformFunction = [this, layer ]( int vi )-> const QgsMeshVertex
Expand Down Expand Up @@ -789,3 +838,8 @@ QgsMeshVertex QgsMeshTransformVerticesByExpression::transformedVertex( QgsMeshLa
else
return layer->nativeMesh()->vertex( vertexIndex );
}

void QgsMeshTransformVerticesByExpression::setZFromTerrain( bool enable )
{
mZFromTerrain = enable;
}
15 changes: 14 additions & 1 deletion src/core/mesh/qgsmeshadvancedediting.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,11 @@ class CORE_EXPORT QgsMeshTransformVerticesByExpression : public QgsMeshAdvancedE
*
* \note this method not apply new vertices to the mesh layer but only store the calculated transformation
* that can be apply later with QgsMeshEditor::advancedEdit()
*
* \param layer
* \param project QgsProject if it is necessary for the calculation ( for example with \see setZFromTerrain() ) \since QGIS 3.44
*/
bool calculate( QgsMeshLayer *layer );
bool calculate( QgsMeshLayer *layer, QgsProject *project = nullptr );

/**
* Returns the transformed vertex from its index \a vertexIndex for the mesh \a layer
Expand All @@ -190,11 +193,21 @@ class CORE_EXPORT QgsMeshTransformVerticesByExpression : public QgsMeshAdvancedE
*/
QgsMeshVertex transformedVertex( QgsMeshLayer *layer, int vertexIndex ) const;

/**
* Sets if Z values for vertices should be obtained from project terrain, instead of expression.
*
* \param enable
*
* \since QGIS 3.44
*/
void setZFromTerrain( bool enable );

private:
QString mExpressionX;
QString mExpressionY;
QString mExpressionZ;
QHash<int, int> mChangingVertexMap;
bool mZFromTerrain = false;

QgsTopologicalMesh::Changes apply( QgsMeshEditor *meshEditor ) override;

Expand Down
19 changes: 8 additions & 11 deletions src/ui/mesh/qgsmeshtransformcoordinatesdockwidgetbase.ui
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@
</property>
</widget>
</item>
<item row="4" column="0" colspan="2">
<item row="4" column="0">
<widget class="QCheckBox" name="mCheckBoxZFromProjectTerrain">
<property name="text">
<string>Get Z value from project terrain</string>
</property>
</widget>
</item>
<item row="5" column="0" colspan="2">
<layout class="QHBoxLayout" name="horizontalLayout">
<property name="sizeConstraint">
<enum>QLayout::SetDefaultConstraint</enum>
Expand Down Expand Up @@ -150,16 +157,6 @@
<item row="1" column="1">
<widget class="QgsExpressionLineEdit" name="mExpressionEditX" native="true"/>
</item>
<item row="5" column="0" colspan="2">
<widget class="QPushButton" name="mGetZValuesButton">
<property name="enabled">
<bool>false</bool>
</property>
<property name="text">
<string>Get Z value from project terrain</string>
</property>
</widget>
</item>
</layout>
</widget>
</widget>
Expand Down
Loading