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

Jump to bottom

Explain how Z values are asssigned to mesh elements #7730

Merged
merged 2 commits into from
Oct 17, 2022
Merged

Explain how Z values are asssigned to mesh elements #7730

merged 2 commits into from
Oct 17, 2022

Conversation

DelazJ
Copy link
Collaborator

@DelazJ DelazJ commented Aug 22, 2022

refs #6883, refs qgis/QGIS#45140
Completes #7729

vcloarec
vcloarec suggested changes Sep 30, 2022
View reviewed changes
docs/user_manual/working_with_mesh/mesh_properties.rst Outdated
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex on a face | --- | Mesh layer | Interpolated from the face vertices |
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex snapped to a 2D vector feature | --- | --- | No value |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| Vertex snapped to a 2D vector feature | --- | --- | No value |
| Vertex snapped to a 2D vector feature | --- | --- | Default or user |

Copy link
Member

Choose a reason for hiding this comment

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

not in master but should fixed by qgis/QGIS#50406

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should it be backported?

Copy link
Member

Choose a reason for hiding this comment

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

The fix should be backported (I just added the label).
If you ask for the doc, maybe it should be backported once the fix will be effectively backported?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was asking for code but your reply made me realize that I didn't backport any editing changes to 3.22 docs. So done now at #7818 (allowing you to review the unreviewed bits I merged last week - I'll forward port your suggestions)

docs/user_manual/working_with_mesh/mesh_properties.rst Outdated
Comment on lines 711 to 715
| Vertex on an unshared edge | No | :guilabel:`Vertex Z value` | Default or user defined |
+ +-------------------------+ +------------------------------------------+
| | Yes | | Average of the selected vertices |
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex on an edge shared by two faces | --- | Mesh layer | Interpolated from the faces' vertices |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| Vertex on an unshared edge | No | :guilabel:`Vertex Z value` | Default or user defined |
+ +-------------------------+ +------------------------------------------+
| | Yes | | Average of the selected vertices |
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex on an edge shared by two faces | --- | Mesh layer | Interpolated from the faces' vertices |
| Vertex on an edge shared by two faces | --- | Mesh layer | Interpolated from the edge' vertices |

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any reason to have removed the "vertex on unshared edge" rows?

Copy link
Member

Choose a reason for hiding this comment

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

For vertex on edge inserted on an edge, the behavior is the same if the edge is shared or not by faces: the value is interpolated between the two vertices at the extremity of the edge.
So, better to remove also "shared by two faces" in line 711.

Copy link
Collaborator Author

@DelazJ DelazJ Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not that sure. See video where numbers represent vertex z value and what happens (new vertex takes value from widget). Peek is bugging here so can't record but if I select vertices before adding the new one, I get the average value assigned.

unsharedEdgeNoselection.mp4

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, may be a bug... I will look at...

DelazJ
DelazJ commented Oct 3, 2022
View reviewed changes
Copy link
Collaborator Author

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

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

Thanks @vcloarec
Few questions below and also, is the table understandable or do you think of any better formatting?

docs/user_manual/working_with_mesh/mesh_properties.rst Outdated
Comment on lines 711 to 715
| Vertex on an unshared edge | No | :guilabel:`Vertex Z value` | Default or user defined |
+ +-------------------------+ +------------------------------------------+
| | Yes | | Average of the selected vertices |
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex on an edge shared by two faces | --- | Mesh layer | Interpolated from the faces' vertices |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any reason to have removed the "vertex on unshared edge" rows?

docs/user_manual/working_with_mesh/mesh_properties.rst Outdated
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex on a face | --- | Mesh layer | Interpolated from the face vertices |
+---------------------------------------+-------------------------+-------------------------------+------------------------------------------+
| Vertex snapped to a 2D vector feature | --- | --- | No value |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should it be backported?

@DelazJ DelazJ merged commit 7ed6dec into qgis:master Oct 17, 2022
@DelazJ DelazJ deleted the meshEditZ branch October 17, 2022 22:04
@qgis-bot qgis-bot mentioned this pull request Oct 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vcloarec vcloarec vcloarec requested changes

Assignees
No one assigned
Labels
Editing Mesh
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DelazJ @vcloarec