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

Mesh point add elevation #58877

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

Conversation

JanCaha
Copy link
Contributor

@JanCaha JanCaha commented Sep 26, 2024

Description

This PR make some changes when adding new vertices to the MeshLayer.

  • newly added vertexes are selected for easier assignment of Z value through the ZValuesWidget
  • a button that assigns Z value from QGIS project's QgsAbstractTerrainProvider to currently selected vertices
  • a checkbox that allows newly added vertices to automatically get Z value from QGIS project's QgsAbstractTerrainProvider, when the point is created

The placement of the new button and checkbox is up for discussion, so far it is added into ZValuesWidget for convenience

@github-actions github-actions bot added this to the 3.40.0 milestone Sep 26, 2024
Copy link

github-actions bot commented Sep 26, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 4d51457)

src/app/mesh/qgsmaptooleditmeshframe.cpp Outdated Show resolved Hide resolved
src/app/mesh/qgsmaptooleditmeshframe.h Outdated Show resolved Hide resolved
src/app/mesh/qgsmaptooleditmeshframe.cpp Outdated Show resolved Hide resolved
src/app/mesh/qgsmaptooleditmeshframe.h Show resolved Hide resolved
src/app/mesh/qgsmaptooleditmeshframe.cpp Outdated Show resolved Hide resolved
@uclaros uclaros added Frozen Feature freeze - Do not merge! Mesh Related to general mesh layer handling (not specific data formats) labels Sep 27, 2024
Comment on lines +2793 to +2807
{
if ( mZValueWidget->getZFromProjectElevationEnabled() )
{
const QgsAbstractTerrainProvider *terrainProvider = QgsProject::instance()->elevationProperties()->terrainProvider();
const QgsCoordinateTransform transformation = QgsCoordinateTransform( mCurrentLayer->crs(), terrainProvider->crs(), QgsProject::instance() );

try
{
const QgsPointXY point = transformation.transform( effectivePoint.x(), effectivePoint.y() );
zValue = terrainProvider->heightAt( point.x(), point.y() );
}
catch ( const QgsCsException & )
{
zValue = std::numeric_limits<double>::quiet_NaN();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@uclaros unlike what we discussed last week, this cannot be moved up in the hierarchy of those if else statements.

I believe that this settings should only affect newly added vertices that are isolated. We probably do not want to affect behavior when adding vertices on edges and in faces (those have Z interpolated from the MESH vertices itself) and that is what the previous if else statements do. We only what to affect the else branch which are the isolated vertices. So I believe that this should be correct.

@JanCaha
Copy link
Contributor Author

JanCaha commented Oct 2, 2024

@uclaros This should be ready, all the review comments are addressed.

Only need to solve the comment above ;-)

@lbartoletti lbartoletti added the Requires Tests! Waiting on the submitter to add unit tests before eligible for merging label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frozen Feature freeze - Do not merge! Mesh Related to general mesh layer handling (not specific data formats) Requires Tests! Waiting on the submitter to add unit tests before eligible for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants