Fix T86926 .
Connect open ends of every edge-net to form a closed region.
Structure member open_v1, open_v2 created to store open end verts.
Before confirming the cut:
After confirming the cut:
Details
Details
- Reviewers
Germano Cavalcante (mano-wii) Campbell Barton (campbellbarton) - Maniphest Tasks
- T86926: Inconsistent behavior of knife tool
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Where the red and yellow color represent the faces.
Thanks for the patch.
Although we can see the edges. This change does not fix the problem.
The knife tool is not supposed to generate loose edges.
In your image, the result that I believe to be correct would be something like this drawing:
Where the red and yellow color represent the faces.
Comment Actions
- Boolean is_closed introduced : checks knife cut is closed or not
if knife cut is closed, join knife cut edge group and boundary edges with previous logic otherwise use first and last vert of knife cut for the connection.
Comment Actions
- New code block added to cast ray in both the directions Assign index to the integer variable index_other of closest BMVert
( casting ray in a single direction frequently produces loose edges )
- New bool direction introduced : to cast the ray in opposite direction to the ray casted from the first knife vert
Comment Actions
I did some tests. The result is still not perfect, but it is much better :)
bm_face_split_edgenet_find_connection is a somewhat complex function, and calling multiple times like this makes the code a little confusing.
Perhaps it would be good to make it clearer in comments what this code is doing.
Personally, I would prefer a new function to be created, something like:
static int bm_face_split_edgenet_find_nearest_non_intersecting_vert_for_connection(const struct EdgeGroup_FindConnection_Args *args, BMVert *v_origin, const short ignore_flag)
Choosing direction depending on what was found before is bad for readability and it doesn’t seem like something that would be planned for an original code.
So, although it improves the knife's behavior, I'm not sure if it's worth lowering the quality of the code in favor to improving such a little-used feature.
So, I am accepting the patch, but I will leave the final decision to @Campbell Barton (campbellbarton)
| source/blender/bmesh/intern/bmesh_polygon_edgenet.c | ||
|---|---|---|
| 1584 | Follow the style guide for new comments: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments | |
Comment Actions
- although this is far less common.
The edge net functions are quite low level and intended to operate on complete nets (without handling loose ends).
I'd rater the knife tool code handles adding extra edges to connect the knife cut up to vertices of the face.
*Edit* this patch assumes the edge-net is a single line with two loose ends (based on a simple vertex/edge count check). Where an edge net can be any number of connected/disconnected edges. So this isn't a good place to resolve the issue.
Having said this, the knife tool can also be used to create arbitrary, disconnected cuts - using the E-key to create new cuts while the knife tool is running, e.g,
So I think it's reasonable for the knife tool to take open loops and create edges connecting them to the face side.
More involved cases like the one above could work as they do now (not very well, however the expected result isn't so obvious either).
Comment Actions
Connect open ends of every edge-net to form a closed region.
- Structure member open_v1, open_v2 created to store open end verts.