See: T78391
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Reminder, even when it seems obvious, short description of what the patch does (for the body of the commit message) should be included.
In this case noting that this completes support for non uniform grids with a reference to the task would be sufficient.
The patch seems fine although we could consider refactoring this into snap_spatial[3], snap_spatial_precise[3] - for XYZ snapping, then have all snap logic index into these vectors (even if they happen not to support non-uniform snap).
It would make expressions like const float iter_fac = ((i == 1) ? grid_dist_y : grid_dist_x) * asp[i]; less awkward.
Although I don't think it's necessary and may have some down-sides, so this patch is fine as-is. Other refactoring could be considered separately.
| source/blender/editors/transform/transform.c | ||
|---|---|---|
| 1724 | Worth a comment that r_snap is used for all axis, unless r_snap_y is set, in that case only the X axis. This is an example where refactoring this to always setup XYZ axes as noted in the reply would be a bit less confusing. | |
Comment Actions
Agreed.. I tried it the other way as well and the diff started to get very brittle. Given this code doesn't get a lot of traffic, I tried to keep the footprint smaller at the risk of some confusion.