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] mesh frame editing part 3 - Edit mesh map tool #44037

Merged
merged 32 commits into from
Jul 14, 2021

Conversation

vcloarec
Copy link
Member

@vcloarec vcloarec commented Jul 2, 2021

UPDATED 2021/08/17

This is the beginning of the implementation of the mesh edit map tool. For now this map tool allow:

Add vertices

  • double clicks on a face : add a vertex in the face, remove the face and replace it by triangles from the surrounding vertices to the new vertex
  • double clicks outside a face: add a "free vertex", that is a vertex not link to any face. This vertex is represented by a red dot when the layer is in editing mode

The Z value of the new vertex is taken from the Z value widget on the top right of the canvas.
If some vertices are selected, the value in the Z value widget is the average value of selected vertices.
Deselecting the vertices allows returning to the default value.

If the user adds a vertex with selected vertices (with the average value in the widget), the new vertex will have this average value for Z.
Keeping pressed the Control key allows maintaining this average value for following new vertices, and this value becomes the new default value.

If there is no selected vertex, the Z value for new vertices is the default one in the Z value widget, or if the new vertex is added to face, the Z value is the interpolated value in the face.

If the vertex is snap on a vector layer and the match point is valid, the Z value will be one of the vector layers.

mapTool_add_vertex

Add faces

  • Left click on the little triangle that appears when the mouse is on a boundary vertex or a free vertex start adding a face.
  • The user has to left click on existing vertex to select the vertex of the new face.
  • Double clicks when digitizing a face add a vertices under the mouse and add it to the face.
  • Backspace remove the last selected vertex from the face.
  • If the area of the rubber band is green, right click valid the face and add it to the mesh. If red, the current face is not valid and can't be added.

Snapping on vector layer is supported.

mapTool_add_faces

Select vertices/faces

  • click on a vertex: select one vertex
  • click on the red box in the face centroid: select all the vertices of a face
  • select vertices by dragging rectangle:
    - Default: select all the partially/touched contained faces
    - Alt modifier: select all the completely contained faces

modifier shift: add new selection to the previous selection
modifier ctrl: remove new selection from the previous selection

mapTool_select vertex

Remove vertices/faces

With a selection of vertices (one or many):
"Ctrl"+"Delete" keys: remove vertices and linked faces and fill hole(s) by a triangulation
"Ctrl"+"Shift"+"Delete" keys: remove vertices and linked faces and do NOT fill hole(s)
"Shift"+"Delete" keys: remove faces without vertices
also available with context menu after right click

removeVertices

Change the Z values of vertices

  • select one or many vertices
  • change the value in the Z value widget,
  • press "Enter" key

mapTool_apply_zvalue

Move vertices

Once some vertices/faces are selected:

  • Press left mouse button on selected vertex, or on the red box in middle of selected faces or edges (faces or edges are selected if all their vertices are selected)
  • hold the button press and drag the mouse cursor
  • release when he moved elements are in a desired position
    The rubber band stays green while the faces are valid, and becomes red if some face are invalid or new position of faces leads to intersect other faces or vertices. If the rubber band is red, elements are not moved if the user release the left mouse button.

mapTool_move_vertices

Interaction with edges

When the mouse hover an edge, the edge is highlight and, depending of the configuration of the edge, some markers appears:

  • a box, clicking on it leads to select extremity vertices.
  • a cross if the two faces on either side can be merged, clicking on it leads to merge the faces.
  • a circle if the edge can be flip, clicking on it leads to flip the edge

mapTool_flip_merge

Split faces

When faces are selected:

  • right click, if there are one or more quad faces, the context menu has a command "Split faces"

mapTool_split

@github-actions github-actions bot added this to the 3.22.0 milestone Jul 2, 2021
@vcloarec vcloarec force-pushed the meshEditMapTool branch 2 times, most recently from f665ea5 to ab511db Compare July 3, 2021 00:10
@vcloarec vcloarec added Feature Mesh Related to general mesh layer handling (not specific data formats) labels Jul 3, 2021
@zacharlie zacharlie added the Changelog Items that are queued to appear in the visual changelog - remove after harvesting label Jul 4, 2021
@vcloarec vcloarec added the Prototype For comment only, do not merge. Avoid using this tag and open a QEP instead! label Jul 5, 2021
@PeterPetrik
Copy link
Contributor

I reviewed it already in the private PR.

@vcloarec
Copy link
Member Author

vcloarec commented Jul 7, 2021

for moving vertices, for now it is possible with another mode than add/remove vertices/faces. I wonder if it will not be better to have the possibility to move in the same mode than add/remove.

@vcloarec
Copy link
Member Author

vcloarec commented Jul 12, 2021

after trying, I have remove the specific button for accessing move vertices functionality. Indeed, I find that it quite frustrating to have to push the tool bar buttons to switch between adding/removing,flip/edge and move vertices.
Now, for moving vertices, the user press mouse button on a selected vertex, or on red box of selected face or on red box of selected edge (bosh vertices selected), and drag.
Text and GIF above updated

@PeterPetrik PeterPetrik self-requested a review July 12, 2021 09:08
Copy link
Contributor

@PeterPetrik PeterPetrik left a comment

Choose a reason for hiding this comment

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

great work.

@vcloarec
Copy link
Member Author

test failing seems unrelated

@vcloarec vcloarec removed the Prototype For comment only, do not merge. Avoid using this tag and open a QEP instead! label Jul 13, 2021
@PeterPetrik
Copy link
Contributor

  1. When I create a new mesh frame (via new dialog), how can I add first face? I can add vertices ( I see number of vertices is increased), but it still says "Invalid Mesh". In the styling dialog, the mesh frame styling is just blank page (can we add some info there rather that unable to style since mesh is invalid?). It would be useful to be able to see only vertices?

  2. segmentation fault when I switched to GitHub and started to type the item 1)

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libqgis_app.3.21.0.dylib      	0x000000010bb694bc QgsZValueWidget::keyboardEntryWidget() const + 12 (qgsmaptooleditmeshframe.cpp:83)
1   libqgis_app.3.21.0.dylib      	0x000000010bb75976 QgsMapToolEditMeshFrame::keyPressEvent(QKeyEvent*) + 678 (qgsmaptooleditmeshframe.cpp:865)
2   org.qgis.qgis3_gui            	0x000000010e8adfd0 QgsMapCanvas::keyPressEvent(QKeyEvent*) + 2288 (qgsmapcanvas.cpp:1807)
3   org.qt-project.QtWidgets      	0x0000000110acbe4f QWidget::event(QEvent*) + 527
4   org.qt-project.QtWidgets      	0x0000000110b7458d QFrame::event(QEvent*) + 45

@PeterPetrik
Copy link
Contributor

  1. I think that for UX it would be better if the elevation widget is dockable Qt widget as everything else in the GUI? Also can we move "Digitize Mesh Elements" button next to "start editing" button..? I am wondering if the "Digitize Mesh Elements" button is needed at all? If your active layer is mesh, and you want to start editing it could start editing right away ?

Screenshot 2021-07-13 at 9 31 34

  1. Can we name the "vertex elevation" dataset "Bed Elevation"?

@saberraz
Copy link
Contributor

saberraz commented Jul 13, 2021

  1. Can we name the "vertex elevation" dataset "Bed Elevation"?

I would change it to Vertex Value to be a more generic term.

@PeterPetrik
Copy link
Contributor

  1. something is not working with the vertices. (same as 1) If I digitise them I cannot see where they are (in case they are outside existing face). I suggest to add "show vertices" and some basic point styling to the styling mesh frame dialog.

Screenshot 2021-07-13 at 9 38 25

wondering if the native mesh + vertex style shouldn't be on by default when you create new mesh?

@PeterPetrik
Copy link
Contributor

  1. If I click on square, it picks the vertex elevation of of the face/vertex/edge. If I click somewhere else (not on edge/vertex, but randomly on face or outside) it picks 0.0. With the double click as digitizing it happens that it resets the vertex tool to 0.0 and then digitise it that way. I am wondering how to solve this. A) The Z value could be interpolated if it is within any face B) it would be maybe good to have some key modifier for "identify/pick the value" for the Z-coord. C) Maybe in the "Vertex elevalion" widget have a button to Pick the Z-val from canvas ?

@PeterPetrik
Copy link
Contributor

  1. it is quite difficult to do any of the editing on touchpad...

@PeterPetrik
Copy link
Contributor

@vcloarec maybe failing test is due to the @nyalldawson recent changes ?

@vcloarec
Copy link
Member Author

  1. Indeed, there is something broken when adding first vertex after creating a new layer from scratch, I look at later.

  2. Do you remember the key that leads to the crash, I can't reproduce.

  3. Inspire by the rotate feature tool for vector layer that I find quite convenient and elegant. Easier for the user to find it and access it. yes digitizing button is needed to come back to the tool after using another, and there will be other tool button for mesh coming.

  4. As Saber suggest, "Vertex Z value" looks good form me.

  5. Outside the digitizing mesh, the vertex is the intersection of edges. Styling vertices is not supported for now, and I think it is another feature. But, indeed showing native edges when starting editing will be good. A new feature to add later.

 A) The Z value could be interpolated if it is within any face
1) it would be maybe good to have some key modifier for "identify/pick the value" for the Z-coord

Yes, good idea, when a key modifier is pressed and in a face --> interpolate the z value, and if outside a face, take the last interpolated (if exist)

C) Maybe in the "Vertex elevalion" widget have a button to Pick the Z-val from canvas ?

Here, I think it is another new feature, we need to fix what is the source of the Z value, other layer.
By the way, the snapping work on Z value of vector layer.

  1. Maybe touchpad is not very good for digitizing work

@vcloarec
Copy link
Member Author

Last commits fix some digitizing issue, set the native mesh always enabled when editing and clarify the Z value for the new vertex.
Text above updated.

@PeterPetrik PeterPetrik merged commit c09443a into qgis:master Jul 14, 2021
@DelazJ DelazJ added the Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. label Aug 18, 2021
@github-actions
Copy link

@vcloarec
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@github-actions
Copy link

@vcloarec
A documentation ticket has been opened at qgis/QGIS-Documentation#6883
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

tomkralidis pushed a commit to tomkralidis/QGIS that referenced this pull request Aug 22, 2021
[mesh] [feature] add mesh map tool to CRUD/digitize actions for mesh frame (vertices, faces)
@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ChangelogHarvested This PR description has been harvested in the Changelog already. Feature Mesh Related to general mesh layer handling (not specific data formats) Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants