Details

Reviewers

Commits

rBffaaa0bcbf47: UV: Edge selection support

Summary

This patch adds edge selection support for UV editing (refer T76545).
Developed as a part of GSoC 2021 project - UV Editor Improvements.

Previously, selections in the UV editor always flushed down to vertices
and this caused multiple issues such as T76343, T78757 and T26676.
This patch fixes that by adding edge selection support for all UV
operators and adding support for flushing selections between vertices
and edges. Updating UV select modes is now done using a separate
operator, which also handles select mode flushing and undo for UV
select modes. Drawing edges (in UV edge mode) is also updated to match
the edit-mesh display in the 3D viewport.

Notes on technical changes made with this patch:

MLOOPUV_EDGESEL flag is restored (was removed in rB9fa29fe7652a).
Support for flushing selection between vertices and edges.
Restored the BMLoopUV.select_edge boolean in the Python API.
New operator to update UV select modes and flushing.
UV select mode is now part of editmesh undo.

TODOs added with this patch:

Edge support for shortest path operator (currently uses vertex path logic).
Change default theme color instead of reducing contrast with edge-select.
Proper UV element selections for Reveal Hidden operator.

Diff Detail

Repository

rB Blender

Branch

arcpatch-D12028 (branched from master)

Build Status

Buildable 20123
Build 20123: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Fix: Free unreleased memory
Fix: Separate case for handling Select Invert operator in UV face select mode

Harbormaster completed remote builds in B16187: Diff 40264.Aug 3 2021, 1:41 PM

Siddhartha Jejurkar (sidd017) edited the summary of this revision. (Show Details)Aug 4 2021, 8:13 PM

Chris Kohl (ckohl_art) added a subscriber: Chris Kohl (ckohl_art).Aug 16 2021, 8:26 AM

Merge branch 'master' into soc-2021-uv-editor-improvements-edge-selection
UV: Edge selection support for UV select split operator
UV: Edge selection support for UV select pinned operator
Fix: Check edge selection only when edges share a common vertex
Code and comment cleanup: UV Invert selection operator
Fix: Make invert select respect sticky modes when using edge select mode
Edge selection support for Edge ring and loop select operator
Merge branch 'master' into soc-2021-uv-editor-improvements-edge-selection
Cleanup: Use utility function for UV face select with sticky
Merge branch 'master' into soc-2021-uv-edge-select-support

Harbormaster completed remote builds in B16435: Diff 40728.Aug 16 2021, 8:41 AM

Siddhartha Jejurkar (sidd017) edited the summary of this revision. (Show Details)Aug 16 2021, 8:47 AM

Siddhartha Jejurkar (sidd017) edited the summary of this revision. (Show Details)Aug 17 2021, 2:43 AM

Merge branch 'master' into soc-2021-uv-edge-select-support
UV: Drawing selected edges

Harbormaster completed remote builds in B16927: Diff 41674.Sep 10 2021, 8:13 PM

Drawing for selected UV edges requires further discussion with devs. For now I have implemented a simple fix that will help users in visually identifying selected and unselected UV edges.
Fix/Changes made to UV drawing in latest patch update :

Vertex count for UV-edge polygon is changed from 4 to 6 in the geometry shader
For UV edges which are not selected, the edge-centers will not be highlighted (orange). This helps in identifying which edges are selected and which are not. Refer screenshots given below for comparison with current master behavior :

Current master - If both vertices of the edge are selected, then the entire edge will be highlighted as selected. This is visually misleading since this is the same result when both vertices of the edge are selected but the edge itself isn't selected

New edge drawing - Edge centers will not be highlighted for selection states, where both vertices of the edge are selected but the edge itself isn't selected

Siddhartha Jejurkar (sidd017) edited the summary of this revision. (Show Details)Sep 11 2021, 11:00 AM

After some testing, I think it would be beneficial to add a new operator to handle UV selection mode flushing, similar to MESH_OT_select_mode.

Isaac Gicheha (Gicheha) added projects: Modeling, Restricted Project, Restricted Project.Oct 27 2021, 6:13 AM

Pratik Borhade (PratikPB2123) awarded a token.Nov 19 2021, 10:44 AM

Fix: Deselecting edges in sticky vertex mode
Merge branch 'master' into soc-2021-uv-edge-select-support
Code cleanup and minor fixes

Harbormaster completed remote builds in B18817: Diff 45006.Nov 19 2021, 7:51 PM

Gavriel Lavi (Gavriel5578) awarded a token.Nov 20 2021, 2:12 AM

Rebase on master (also some spelling corrections).

Harbormaster completed remote builds in B19987: Diff 47059.Jan 14 2022, 4:18 AM

Simplify edge-selection drawing, matching edit-mesh display in the 3D view port.

Don't display vertices.
Don't blend the selection color.
Use a define instead of run-time GLSL changes (for alternate behavior with edge-selection enabled).

Harbormaster completed remote builds in B19988: Diff 47060.Jan 14 2022, 5:30 AM

Minor edits to comments.

Harbormaster completed remote builds in B19989: Diff 47061.Jan 14 2022, 6:27 AM

Minor tweaks to comments.

Harbormaster completed remote builds in B19990: Diff 47062.Jan 14 2022, 7:01 AM

Edge support for UV Rip tool
Edge support for Unwrap helper functions
Comment cleanup

Harbormaster completed remote builds in B20123: Diff 47369.Jan 24 2022, 9:01 AM

Some notes on this patch:

In general I find the some of these flushing checks in this patch difficult to follow.

Unless there are good reasons to diverge from BMesh logic regarding flushing and the selection checks - I think it would be simplest to follow their convention.

For example, currently checking if a face is selected always checks vertices and edge selection state. Where as for BMesh the edge checks are only done when in edge selection mode.

It would be good to see if some of the low-level flushing or looping over UV's could be moved into functions, there seems to be too many inland loops doing low level operations.

We might consider moving the sticky setting into the tools settings as it will avoid having to pass the image space around to low level selection functions. This makes uv_select_all_perform for example read poorly as invert requires the image-space to be passed where select all/none doesn't. Short term (part of this patch) select all/none could be split into a separate function. Longer term we might be better off moving the sticky setting. I don't think it's important to uses that this be a space-setting instead of a tool setting.

source/blender/editors/uvedit/uvedit_select.c
468	`use_mesh_location` is misleading, as the 3D mesh location isn't being checked, it's checking the UV's are connected to the same vertex. (there might be vertices that share a location which will be ignored for example).
683	Suggest `uvedit_uv_select_shared_vert`
2180–2203	This is quite low level logic, could some of it be moved into a more general utility function?

Campbell Barton (campbellbarton) added reviewers: Brecht Van Lommel (brecht), Campbell Barton (campbellbarton).Jan 24 2022, 10:15 AM

Benjamin Sauder (kioku) added a subscriber: Benjamin Sauder (kioku).Jan 24 2022, 3:53 PM

Martijn Versteegh (Baardaap) added a subscriber: Martijn Versteegh (Baardaap).Jan 24 2022, 9:34 PM

Edge support for UV Hide selected operator
Edge support for UV Reveal hidden operator
Minor code and comments cleanup

Harbormaster completed remote builds in B20170: Diff 47449.Jan 25 2022, 6:26 PM

Rewrite: Follow BMesh logic for UV selection
Rewrite: Follow BMesh logic for UV flushing and selection checks
Add new operator for setting UV select mode and flushing selections based on select mode and sticky mode
Undo for UV select mode

Harbormaster completed remote builds in B20245: Diff 47758.Jan 31 2022, 7:02 AM

Siddhartha Jejurkar (sidd017) edited the summary of this revision. (Show Details)Jan 31 2022, 7:32 AM

Rebase on master and minor code refactor

Harbormaster completed remote builds in B20253: Diff 47779.Jan 31 2022, 8:03 PM

Setting to keep out of review queue for now. A few things still need to be taken care of :

After latest updates, UV operators need to be verified again to ensure functionality was not broken. (ex: rip tool)
Splitting code into general utility functions
Making sure BMesh logic is followed for UV selections wherever possible. (certain exceptions arise due to sticky modes).

Till then, user/developer feedback on the implemented features and improvements in this patch are welcome.

Cleanup based on Campbell's feedback
Finish UV_OT_select_mode - Keeps UV selection states consistent with UV select mode
Fix: Broken sync selection for UV loop and ring select operators
Rewrite: Follow BMesh logic for UV selection flushing
Restrict select pinned operator to UV vertex mode when sync select disabled
Rebase on master

Harbormaster completed remote builds in B20549: Diff 48336.Feb 14 2022, 9:12 PM

Siddhartha Jejurkar (sidd017) added inline comments.Feb 14 2022, 9:25 PM

source/blender/editors/uvedit/uvedit_select.c
468	Changed to `sticky_flag`, which uses existing enums from `eSpaceImage_Sticky`
2180–2203	Moved to `uvedit_vert_is_all_other_faces_selected()`

In D12028#369319, @Campbell Barton (campbellbarton) wrote:

Unless there are good reasons to diverge from BMesh logic regarding flushing and the selection checks - I think it would be simplest to follow their convention.

For example, currently checking if a face is selected always checks vertices and edge selection state. Where as for BMesh the edge checks are only done when in edge selection mode.

Rewrote this patch to follow BMesh logic as seen in bmesh_marking.c :

For checking UV selection states of vertices, edges and faces, do vertex checks only in UV vertex mode
For checking UV selection states of edges and faces, do edge checks only in UV edge, face and island mode
Add functions for UV selection flushing (mode dependent and independent)
Use a separate operator (UV_OT_select_mode) to set UV select mode and clean UV selection states (similar to EDBM_selectmode_set).

It would be good to see if some of the low-level flushing or looping over UV's could be moved into functions, there seems to be too many inland loops doing low level operations.

Moved parts of the code into separate functions :

uvedit_vert_is_edge_select_any_other
uvedit_vert_is_face_select_any_other
uvedit_vert_is_all_other_faces_selected
uv_select_flush_from_loop_edge_flag

This makes uv_select_all_perform for example read poorly as invert requires the image-space to be passed where select all/none doesn't. Short term (part of this patch) select all/none could be split into a separate function.

Agreed. I have split the select all/none and invert logic into separate functions in the latest update.
To make things more simple, maybe we could pass sticky mode as a char or int parameter to the function instead?

Move the undo push out of ED_uvedit_selectmode_clean_multi

RNA updates shouldn't be doing their own undo push calls.

Harbormaster completed remote builds in B20582: Diff 48407.Feb 16 2022, 3:19 AM

Pass const SpaceImage
rename uv_invert_selected -> uv_select_invert (matching uv_select_all)

Harbormaster completed remote builds in B20583: Diff 48408.Feb 16 2022, 3:27 AM

Rebase on master (Sticky setting now moved into tool settings)
Merge back select all/invert since the only reason to split was the requirement for passing in the image space (which is no longer the case).

Harbormaster completed remote builds in B20584: Diff 48409.Feb 16 2022, 4:19 AM

Accepting, with some notes.

Patch description should be modified to make it suitable for a commit message: https://wiki.blender.org/wiki/Style_Guide/Commit_Messages
Remaining TODO's don't seem major but should be listed in the patch description.

This revision is now accepted and ready to land.Feb 16 2022, 4:21 AM

Campbell Barton (campbellbarton) removed a reviewer: Brecht Van Lommel (brecht).Feb 16 2022, 4:22 AM