Fix #121004: bmesh.ops.dissolve_limit wrong in certain cases #121033

Philipp Oeser · 2024-04-24T16:54:34+02:00

Philipp Oeser commented

2024-04-24 16:54:34 +02:00

Exposed by 6c774feba2

BM_mesh_decimate_dissolve_ex sets up DelimitData with layer offset
(start), size and end so that bm_edge_is_contiguous_loop_cd_all can
check a range of edges for being contiguous.

The way cd_loop_offset_end is calculated is wrong though, it does not
take the actual start into account (this has to be added to fix the
bug). When it is wrong, it can happen that start and end are the same,
so no check actually takes place and no delimiting edges are found.

It seems that prior to 6c774feba2 the customdata layer always had an
offset of zero, so never really showed in practice (at least I couldnt
make it break in 3.4), but after 6c774feba2 we can at least observe the
following:

when creating a bmesh, an offset would to the uv layer would still be
zero in my tests
however, as soon as we iterate loops of a face (as done in the
report), we get an additional layer CD_BM_ELEM_PYPTR
this then changes the offset
BM_uv_map_get_offsets_from_layer seems to do the right thing afaict

So to resolve, just add the "start" offset to the end, to get the right
range.

NOTE: there is a very similar DelimitData used in
bmesh.ops.join_triangles and the way in which bm_edge_delimit_cdata
sets up te range is exactly like what this PR proposes.

Exposed by 6c774feba2 `BM_mesh_decimate_dissolve_ex` sets up `DelimitData` with layer offset (start), size and end so that `bm_edge_is_contiguous_loop_cd_all` can check a range of edges for being contiguous. The way `cd_loop_offset_end` is calculated is wrong though, it does not take the actual start into account (this has to be added to fix the bug). When it is wrong, it can happen that start and end are the same, so no check actually takes place and no delimiting edges are found. It seems that prior to 6c774feba2 the customdata layer always had an offset of zero, so never really showed in practice (at least I couldnt make it break in 3.4), but after 6c774feba2 we can at least observe the following: - when creating a bmesh, an offset would to the uv layer would still be zero in my tests - however, as soon as we iterate loops of a face (as done in the report), we get an additional layer `CD_BM_ELEM_PYPTR` - this then changes the offset - `BM_uv_map_get_offsets_from_layer` seems to do the right thing afaict So to resolve, just add the "start" offset to the end, to get the right range. NOTE: there is a very similar `DelimitData` used in `bmesh.ops.join_triangles` and the way in which `bm_edge_delimit_cdata` sets up te range is exactly like what this PR proposes.

Philipp Oeser added 1 commit 2024-04-24 16:54:44 +02:00

f16849ee69 Fix #121004 : bmesh.ops.dissolve_limit wrong in certain cases

Exposed by 6c774feba2

`BM_mesh_decimate_dissolve_ex` sets up `DelimitData` with layer offset
(start), size and end so that `bm_edge_is_contiguous_loop_cd_all` can
check a range of edges for being contiguous.

The way `cd_loop_offset_end` is calculated is wrong though, it does not
take the actual start into account (this has to be added to fix the
bug). When it is wrong, it can happen that start and end are the same,
so no check actually takes place and no delimiting edges are found.

It seems that prior to 6c774feba2 the customdata layer always had an
offset of zero, so never really showed in practice (at least I couldnt
make it break in 3.4), but after 6c774feba2 we can at least observe the
following:
- when creating a bmesh, an offset would to the uv layer would still be
zero in my tests
- however, as soon as we iterate loops of a face (as done in the
report), we get an additional layer `CD_BM_ELEM_PYPTR`
- this then changes the offset
- `BM_uv_map_get_offsets_from_layer` seems to do the right thing afaict

So to resolve, just add the "start" offset to the end, to get the right
range.

NOTE: there is a very similar `DelimitData` used in
`bmesh.ops.join_triangles` and the way in which `bm_edge_delimit_cdata`
sets up te range is exactly like what this PR proposes.

Philipp Oeser added this to the Modeling project 2024-04-24 16:54:59 +02:00

Philipp Oeser added the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #121004: bmesh.ops.dissolve_limit wrong in certain cases #121033