Cleanup: Simplify UDIM parameters when UV packing.
ClosedPublic
Actions

Authored by Chris Blackbourn (chrisbblend) on Nov 24 2022, 1:48 AM.

Details

Reviewers

Siddhartha Jejurkar (sidd017)
Campbell Barton (campbellbarton)

Summary

Migrate (some) of the UDIM offset calculation from inside one of the packing engines (where it's consumed) to the packing operator (where it's produced).

This change (and others) will help simplify the future migration of the packing engine inside the UVEditor (editors/uvedit/uvedit_islands.cc) to the Geometry module, so it can eventually replace the other packer in geometry/intern/uv_parametrizer.cc.

Diff Detail

Repository: rB Blender

Event Timeline

Chris Blackbourn (chrisbblend) requested review of this revision.Nov 24 2022, 1:48 AM

Chris Blackbourn (chrisbblend) created this revision.

Patch should include brief description of how this improves the code.

From a quick check it's not so obvious why this is an improvement.

Chris Blackbourn (chrisbblend) edited the summary of this revision. (Show Details)Dec 6 2022, 3:24 AM

Siddhartha Jejurkar (sidd017) added inline comments.Dec 6 2022, 7:10 AM

source/blender/editors/uvedit/uvedit_islands.cc
735	For future reference, it could be difficult to understand which case is implied when `udim_params != NULL` . Here it means the Closest UDIM option but readers could make the mistake of assuming the Active UDIM option as well. A comment explaining how different cases are handled can be helpful here.

More explicit about closest_udim method.

Campbell Barton (campbellbarton) accepted this revision.Dec 7 2022, 12:56 AM

Campbell Barton (campbellbarton) added inline comments.

source/blender/editors/include/ED_uvedit.h
353	Since this member is no longer in a `UDIM` struct, prefer this be explicit: e.g. `udim_base_offset` then if other UDIM variables are needed they complete usefully and unambiguous.
source/blender/editors/uvedit/uvedit_unwrap_ops.c
1079	picky when using this kind of assignment, I think it reads better to give the variable you intend to pass the short/concise name, then give the variable on the stack a more obscure name - since you never want to use this by accident. struct UVMapUDIM_Params _closest_udim_buf; struct UVMapUDIM_Params *closest_udim = NULL; ... closest_udim_buf = &_closest_udim_buf; closest_udim_buf->.. = ..; ... etc ... (underscore prefix could be left off, just example).