gsoc2024-UV-Seam-Weld #123704

Open
Anish-Bharadwaj wants to merge 23 commits from Anish-Bharadwaj/blender:gsoc2024-UV-Seam-Weld into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
First-time contributor

This PR request demonstrates the implementation of the UV-Seam Weld functionality, and I have included some images showing how it alters UVs. Currently, this implementation only combines two continuous selections of UV coordinates. If more than two are selected, it will still only weld two of the line selections. Additionally, if one line selection has more vertices than the other, it will only merge the minimum number of vertices between the two line selections.

I have tested the functionality under the following conditions:
UV islands from the same mesh
UV islands from different meshes
UV islands with flipped normals

When internal vertices are selected in a UV island, they can also be included in the seam-weld operation. However, I have ensured that no crashes occur in this case.

This PR request demonstrates the implementation of the UV-Seam Weld functionality, and I have included some images showing how it alters UVs. Currently, this implementation only combines two continuous selections of UV coordinates. If more than two are selected, it will still only weld two of the line selections. Additionally, if one line selection has more vertices than the other, it will only merge the minimum number of vertices between the two line selections. I have tested the functionality under the following conditions: UV islands from the same mesh UV islands from different meshes UV islands with flipped normals When internal vertices are selected in a UV island, they can also be included in the seam-weld operation. However, I have ensured that no crashes occur in this case.
Anish-Bharadwaj added 8 commits 2024-06-25 05:30:30 +02:00
Implemented the constructor for the stich by distance operator in the UV_OT_stitch_distance function. Added necessary UI elements for the UV stitch by distance functionality. Set up UI layout and controls for user interaction with the tool.
Currently the Seam weld is able to:
1. group loops based of underlying vertexID
2. Correlate prev and next loop to each loop if they are selected
2. Find endpoints of each continuous selection

Issues:
1. Need to be able to separate loops that have same underlying vertexID
   but different location
instead of mesh vertex ID
Seam-Weld can now still work under following conditions:
1. Can Merge Islands that have flipped normals
2. Can Merge Islands from different Objects
3. Can Merge Islands with different length line segments selections
4. Can Merge when there are more than 2 line segments selected

Seam-Weld not does not crash under the following conditions:
1. When Internal Loops are selected
Campbell Barton requested changes 2024-06-25 05:55:40 +02:00
Campbell Barton left a comment
Owner

Checked and submitted a first pass review however I have some concerns that this approach is over complicated and could make use of more of Blender's internal structures. I'd like to go over this with Anish to check on the method used here and possible alternatives.

Checked and submitted a first pass review however I have some concerns that this approach is over complicated and could make use of more of Blender's internal structures. I'd like to go over this with Anish to check on the method used here and possible alternatives.
@ -675,6 +675,7 @@ void minmax_v2v2_v2(float min[2], float max[2], const float vec[2]);
/** ensure \a v1 is \a dist from \a v2 */
void dist_ensure_v3_v3fl(float v1[3], const float v2[3], float dist);
void dist_ensure_v2_v2fl(float v1[2], const float v2[2], float dist);
float dist_v2v2(const float v1[2], const float v2[2]);

No need for this function, use len_v2 or better len_squared_v2 and compare with a squared length (to avoid square roots which are expensive).

No need for this function, use `len_v2` or better `len_squared_v2` and compare with a squared length (to avoid square roots which are expensive).

Use len_v2v2 instead. This function isn't needed.

Use `len_v2v2` instead. This function isn't needed.
@ -291,2 +294,4 @@
};
/* `uvedit_smart_stitch.cc` */
struct loopData {

There is no need for this struct to be public (it can be moved to the source file).

Suggest to move all the logic into uvedit_ops.cc since this implementation doesn't seem to share logic with smart-stitch.

There is no need for this struct to be public (it can be moved to the source file). Suggest to move all the logic into `uvedit_ops.cc` since this implementation doesn't seem to share logic with smart-stitch.
@ -293,0 +318,4 @@
return lhs.first == rhs.first && lhs.second == rhs.second;
}
};
void getBMLoopPointers(Scene* scene,

These functions all seem much to low level to be exposed in a public header.

These functions all seem much to low level to be exposed in a public header.
@ -228,0 +229,4 @@
// Function to construct an array of pairs of UV coordinates that map to the UV coordniates along 2 selected line segments

Use C-style comments for everything except disabled code.
https://developer.blender.org/docs/handbook/guidelines/c_cpp/#cc-comments

Use C-style comments for everything except disabled code. https://developer.blender.org/docs/handbook/guidelines/c_cpp/#cc-comments
@ -228,0 +236,4 @@
for (auto it = loopMapPtr->begin(); it != loopMapPtr->end(); ++it) {
const std::pair<float, float>& UVloc = it->first;
loopData& value = it->second;
std::string key = std::to_string(value.objectindex) + "," + std::to_string(value.islandindex);

Converting to a string seems strange and unnecessary... surely and int pair can be used instead.

Converting to a string seems strange and unnecessary... surely and int pair can be used instead.
@ -228,0 +237,4 @@
const std::pair<float, float>& UVloc = it->first;
loopData& value = it->second;
std::string key = std::to_string(value.objectindex) + "," + std::to_string(value.islandindex);
if (value.connec1 == std::make_pair(-1.0f, -1.0f) || value.connec2 == std::make_pair(-1.0f, -1.0f)) {

blender::float2 should be used instead of std::make_pair(float, float).

`blender::float2` should be used instead of `std::make_pair(float, float)`.
@ -228,0 +324,4 @@
aspect_y,
offsets);
int islandcounter = 0;
LISTBASE_FOREACH_MUTABLE (FaceIsland *, island, &island_list) {

If the purpose of this function is to consume all islands, it's more straightforward to write:

while ((island = BLI_pophead(&island_list))) {..}

If the purpose of this function is to consume all islands, it's more straightforward to write: `while ((island = BLI_pophead(&island_list))) {..}`
@ -228,0 +375,4 @@
blender::FunctionRef<void(float[2], float[2], float[2])>user_fn)
{
// Grabs the offset for each respective object that the UV coordinates belong to
const BMUVOffsets offset1 = BM_uv_map_get_offsets(BKE_editmesh_from_object((*objects)[UVcoord1->objectindex])->bm);

Calling BM_uv_map_get_offsets should only be called once per object (ideally), and definitely avoided in functions which are called many times.

Calling `BM_uv_map_get_offsets` should only be called once per object (ideally), and definitely avoided in functions which are called many times.
@ -2789,1 +2789,4 @@
}
static bool uvedit_uv_threshold_weld(Scene *scene,
blender::Vector<Object *> *objects,

Since the vector isn't being manipulated, a more generic type would be better. const Span<Object *> objects.

Since the vector isn't being manipulated, a more generic type would be better. `const Span<Object *> objects`.
@ -2790,0 +2811,4 @@
}
} else{

Use clang-format, strongly recommend to configure your editor to run this on file-save.

Use clang-format, strongly recommend to configure your editor to run this on file-save.
Iliya Katushenock added this to the Modeling project 2024-06-25 14:37:18 +02:00
Iliya Katushenock added the
Interest
UV Editing
label 2024-06-25 14:37:22 +02:00
Anish-Bharadwaj added 4 commits 2024-06-28 09:19:43 +02:00
Added the initial files and connections for the New API that will grab all loop
in continuous selection. Additional Files are going to have changes because Clang Formatting is now being properly enforced.
This API returns an loops that are first grouped by connected line segment and
then grouped by common UV coordinate given same underlying vertex. The
index of all loops will be -1 except for endpoint loops who will have an
index of 1
bmesh_edgeloop_uv API
The Seam weld should not cause any crashes and the only scenarios where
it wont perform a weld is when only one edge loop is selected and if
any of the selected edge loops are cyclic
Campbell Barton requested changes 2024-07-05 02:33:51 +02:00
@ -0,0 +1,81 @@
#include "bmesh_edgeloop_uv.hh"

Misses SPDX license header.

Misses SPDX license header.
@ -0,0 +25,4 @@
continue;
}
std::vector<std::vector<BMLoop *>> edgeloop;
// while the queue is not empty

Use C-style comments for descriptive text. See: https://developer.blender.org/docs/handbook/guidelines/c_cpp/
Also use proper sentences (capitalized, end with a fullstop)

Use C-style comments for descriptive text. See: https://developer.blender.org/docs/handbook/guidelines/c_cpp/ Also use proper sentences (capitalized, end with a fullstop)
@ -0,0 +38,4 @@
BMVert *queuev = queuel->v;
// vector of current UV coord
std::vector<BMLoop *> uvcoord;

The name uvcoord is a bit misleading, perhaps loops_at_uv

The name `uvcoord` is a bit misleading, perhaps `loops_at_uv`
@ -0,0 +1,31 @@
#pragma once

Misses SPDX license header.

Misses SPDX license header.
@ -0,0 +1,31 @@
#pragma once
#include "BKE_customdata.hh"
#include <set>
#include <tuple>

Not needed.

Not needed.
@ -0,0 +4,4 @@
#include <tuple>
#include <unordered_map>
struct ARegion;

Many of these seem not to be needed.

Many of these seem not to be needed.
@ -293,0 +296,4 @@
/* `uvedit_smart_stitch.cc` */
struct loopData {
std::vector<BMLoop *> loops;
std::pair<float, float> connec1 = std::make_pair(-1.0f, -1.0f);

Use float2 instead of std::pair<float, float>.

Use `float2` instead of `std::pair<float, float>`.
@ -293,0 +319,4 @@
return lhs.first == rhs.first && lhs.second == rhs.second;
}
};
void getBMLoopPointers(

This function doesn't seem to be used anywhere.

This function doesn't seem to be used anywhere.
@ -227,0 +227,4 @@
void ED_uvedit_shift_pair_of_UV_coordinates(
BMUVOffsets offset1,
BMUVOffsets offset2,
std::vector<BMLoop *> *UVcoord1,

Prefer blender::Vector data type over std::verctor.

In this case it looks like you could pass in a blender::Span<BMLoop*> which can be passed in with a blender::Vector as the underlying type.

Prefer `blender::Vector` data type over `std::verctor`. In this case it looks like you could pass in a `blender::Span<BMLoop*>` which can be passed in with a blender::Vector as the underlying type.
Anish-Bharadwaj added 8 commits 2024-07-11 09:59:59 +02:00
Campbell Barton reviewed 2024-07-12 02:41:49 +02:00
@ -0,0 +68,4 @@
}
}
}
if (selectedconnections < 2) {

Shouldn't selectedconnections always terminate when it's not 2? Since its not a loop the there is are multiple paths to traverse down. (any UV with 3+ connected selected edges).

Shouldn't `selectedconnections` always terminate when it's not 2? Since its not a loop the there is are multiple paths to traverse down. (any UV with 3+ connected selected edges).
Anish-Bharadwaj added 2 commits 2024-07-12 09:48:08 +02:00
convention
Seam Weld

Now Seam Weld always ensures that the user has made 2 continuous
selections both of which need to be edgeloops. Also validates that UV
coordinates have been welded prior to redrawing UVs.
Anish-Bharadwaj added 1 commit 2024-07-12 10:37:45 +02:00
Now UV_get_edgeloops ignores invalid edgeloops instead of exiting
function. And uvedit_uv_threshold_weld now exits if it encounters cyclic
edgeloop.
Merge conflict checking is in progress. Try again in few moments.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u gsoc2024-UV-Seam-Weld:Anish-Bharadwaj-gsoc2024-UV-Seam-Weld
git checkout Anish-Bharadwaj-gsoc2024-UV-Seam-Weld
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#123704
No description provided.