Fix T84551: Linear subdivisions crashing with geometry loose geometry
Needs RevisionPublic
Actions

Authored by Pablo Dobarro (pablodp606) on Jan 29 2021, 10:13 PM.

Details

Reviewers

Sergey Sharybin (sergey)

Maniphest Tasks

T84551: Multiresolution - Crash when Linear Subdivision level is above 1

Summary

The issue with the reported mesh is that it has a loose edge. When
building the geometry, this was not taken into accout and it was adding
loose geometry to reshape_smooth_context. This was causing
openSubdiv_createTopologyRefinerFromConverter to return null.

For linear subdivision to work, all internal faces edges and edges
adjacent to a real edge need to be added, but loose edges need to be
skipped.

Diff Detail

Repository

rB Blender

Branch

T84551 (branched from master)

Build Status

Buildable 12484
Build 12484: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) created this revision.Jan 29 2021, 10:13 PM

Harbormaster completed remote builds in B12484: Diff 33325.Jan 29 2021, 10:13 PM

Pablo Dobarro (pablodp606) requested review of this revision.Jan 29 2021, 10:13 PM

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)Feb 1 2021, 3:14 AM

Pablo Dobarro (pablodp606) added a reviewer: Sergey Sharybin (sergey).

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)

From the sound of the description of the problem, the proper and more simple approach is to move /* Ignore all loose edges as well, as they are not communicated to the OpenSubdiv. */ before the simple subdivision mode check.

source/blender/blenkernel/intern/multires_reshape_smooth.c
796–800	This seems to be wrong. All edges are to be marked as sharp, regardless of whether they are inner or not.

This revision now requires changes to proceed.Feb 1 2021, 10:02 AM

I need to try this again. If I'm understanding this correctly, this should work the following way:

If it is a loose edge, always return.
If linear subdivisions are used, mark everything as sharp (inner and coarse)
If simple subdivisions are used, mark only coarse as sharp
For normal subdivisions, mark with the corresponding crease value.

In that case, I think there was something undefined in how the code was working before. If coarse_edge_index is equal to ORIGINDEX_NONE (in this case, -1), it will use the -1 index to check the bitmap. I think this was causing all inner edges to be considered loose geometry in my tests. That also needs to be fixed, right?

If it is a loose edge, always return.
If linear subdivisions are used, mark everything as sharp (inner and coarse)
If simple subdivisions are used, mark only coarse as sharp
For normal subdivisions, mark with the corresponding crease value.

This seems to be correct. If you put if() statements in the same order, the conditions will be simple and easy to follow. You can also add this bulletlist in the comment. Can't be too explicit!

In that case, I think there was something undefined in how the code was working before. If coarse_edge_index is equal to ORIGINDEX_NONE (in this case, -1), it will use the -1 index to check the bitmap. I think this was causing all inner edges to be considered loose geometry in my tests. That also needs to be fixed, right?

This is not impossible and indeed needs fixing!

Revision Contents
Changeset List

Path

Size

source/

blender/

blenkernel/

intern/

multires_reshape_smooth.c

11 lines

Diff 33325

View Options

source/blender/blenkernel/intern/multires_reshape_smooth.c

Fix T84551: Linear subdivisions crashing with geometry loose geometryNeeds RevisionPublicActions