Page MenuHome

Solidify with fill 'only rim' results in wrong face loop cycle for certain topology
Closed, ResolvedPublic

Description

System Information
Windows 8.1, NVIDIA GeForce GTX 645

Blender Version
Broken: 2.73a

When I use Rip on plane with Solidify and Bever modifiers applied, Blender crashes.

Exact steps for others to reproduce the error
Create plane and subdivide once. Add Bevel and Solidify modifiers (Solidify on top). Check Only Rim from Solidify modifier. Select middle verticle and rip (V). Use Rip again and Blender will crash.

Just open attachment rip_crash.blend and press V.

Event Timeline

Jukka Martama (lawnmower) set Type to Bug.
Jukka Martama (lawnmower) created this task.
Jukka Martama (lawnmower) raised the priority of this task from to Needs Triage by Developer.

can confirm crash in Blender 2.73 on Ubuntu 14.04.1.

very interesting, will try to investigate.

Niko Leopold (mangostaniko) triaged this task as Normal priority.Feb 13 2015, 1:19 AM

it is obiously not a problem of the rip tool per se, but more generally of the mesh structure.
i tried to simplify the mesh as much as possible to leave only the essential topology (see image below).

when applying solidify with fill 'only rim' to the simplified mesh, obviously one face gets a wrong loop edge assignment in its loop cycle.

bmesh_elem_check then fails with error codes
2^20 (loop base vertex or next loop base vert are not the vertices of their edge.), and
2^21 (bmesh_radial_validate failed, loop base vertex of some loop in the radial cycle
— i.e. among the loops around the same shared edge — doesnt belong to the edge.)

and when the bevel modifier is being drawn, blender crashes from segmentation fault due to nullpointer after some geometry could not be found in the generation of the beveled faces.

so the real issue seems to lie in the solidify modifier, since when modelling the mesh manually, all bmesh assertions pass and everything works fine.
so will try to investigate into what happens on solify application.

Niko Leopold (mangostaniko) renamed this task from Rip crashes with solidify and bever modifiers to Solidify with fill 'only rim' results in wrong face loop cycle for certain topology .

note that as a workaround you can select the mesh and run C.active_object.data.validate() in the python interpreter to fix the mesh. if it returns True it actually changed something.

ok so from the comments in MOD_solidify.c /* 3+ faces using an edge, we can't handle this usefully */
and from the report cited below it seems that this is a more general and known issue (just try any edge with more than 2 faces attached, using fill only rim, it will be the same issue).

Solidify modifier just doesnt work well when 3+ faces use an edge, this is a known limit with the method used.
Its designed not to fail outright, but not really supported either.
[...] its possible though difficult to integrate with the current modifier.

so at this point unfortunately i must say that i dont have sufficient studied topology and dont really have the time to try a rewrite of the modifier.

but it would be nice to at least be able to assert the above statement 'Its designed not to fail outright', since when we use a bevel modifier on it (and likely in other cases), it rather segfaults outright due to the invalid mesh structure.

a dirty hack would be to just run mesh_validate after the modifier is applied with the only rim option, where the issue is known to occur. but thats not really beautiful.
guess we might ask @Campbell Barton (campbellbarton) at this point.

This is caused by D737, the problem is that the number of newVerts is being confused with the newEdges.

When each vertex is on the boundary of only one face island. These values are the same. but when vertices are shared you get this:

+----+
|    |
|    |
+----+----+
     |    |
     |    |
     +----+
    
there should be: 7 verts, 8 edges.

... currently in this case it only allocates 7 edges. when it should be 8.

ok so i see that the number of edges to use in CDDM_from_template is

(int)((numEdges * stride) + newEdges + newVerts)

in case of do_shell stride is 2 and newVerts is 0, since it just takes a translated copy of the original mesh and adds the newEdges of the rim.

in the case of no shell (only rim) the stride is 1 and newVerts is assigned the value of newEdges, as if assuming that for each vertex at the end of the new extruded edges there is one edge on the rim.
and as you pointed out, this assumption is wrong.

hm.. i wonder about the we can't handle this usefully */ comment. i guess a lot has been tried before, but if it can't be handled without a extensive rewrite of the modifier, maybe still the crash can be avoided?
im just wondering if maybe the code from the do_shell could be used and then just cut out the shell, or maybe even use some of the code from mesh_validate.c (maybe count the islands and find a pattern to get the edge count if there is one, but well probably its not just about the number of edges), but i fear that my oversimplifications leave out a lot of the variables involved...

btw. sorry for thinking out loud, i guess thats rather unappropriate...
though this issue seems too involved for me atm. im learning a lot in all this process, but i shouldnt go into such detail i feel.

@Martin Felke (scorpion81) is going to check on the bug.

@Niko Leopold (mangostaniko), its fine to show results of investigation, this is rather hard to follow in areas.
However the comment ... we can't handle this usefully */ isn't relevant. Thats related to normal calculation and not even executing in this crash.

seems like i lost the momentum too early...
thanks for the fix, i will study the solution carefully :)