Page MenuHome

Sculpt mode with Mask modifier enabled result an instant crash
Closed, ResolvedPublic


Note, crash is resolved, but normal calculation logic looks like it may not be correct still. (though it doesn't result in visible errors, just extra calculations).

System Information
Win 7 64 bit

Blender Version
Broken: 2.76b official and build c986e93

As the title says; If you add a Mask modifier to a mesh (with a vertex group assigned), then switch to Sculpt mode...Blender will always crash for me. If you disable the modifier, switch to Sculpt mode and then re-enable the'll crash once you start sculpting.

Exact steps for others to reproduce the error

: Just switch to Sculpt mode .

Event Timeline

Mohammad Eideh (quacker) raised the priority of this task from to Needs Triage by Developer.
Mohammad Eideh (quacker) updated the task description. (Show Details)
Mohammad Eideh (quacker) set Type to Bug.
Campbell Barton (campbellbarton) lowered the priority of this task from Needs Triage by Developer to Confirmed, Medium.Jan 12 2016, 7:59 PM

Crash confirmed, will have a look later.

Looked into some different fixes.

At first I looked into passing mapping data to BKE_pbvh_update (pass polygon CD_ORIGINDEX and dm->numPolyData), then use this to array to only fill in the subset of faces used in the final CDDM (needs the reverse of the orig-index array). In this report the orig-index data isn't available (although it could be required and made available if it was important).

On further investigation I couldn't see why this call is even necessary, commenting all calls to cdDM_update_normals_from_pbvh, works well on my tests, with this report and with the file from T46726.

@Antony Riakiotakis (psy-fi), any hints on what case this is needed for?

Assuming I missed some test case and this call is needed:

This is a possible fix: P313: only update normals on deform derived meshes since constructive modifiers wont have aligned arrays.

Further issues

  • When testing this report, I noticed you can sculpt onto invisible parts of the mesh (not a new bug, checked 2.71, 2.75 and 2.76 releases).
  • cdDM_update_normals_from_pbvh makes BKE_pbvh_update is running every redraw, where its mostly finding 0 nodes, yet it still runs pbvh_update_normals and allocates vertex array... which is never used (simple to return when totnode == 0, but probably best this bug gets sorted out first).

cdDM_update_normals_from_pbvh makes sure normals are updated even when we don't use optimal drawing from pbvh. It basically flushes the changes from PBVH to the derivedmesh itself. It's mostly useful on non-solid draw mode or when we can't do optimized drawing with PBVH (check can_pbvh_draw function)

Note - if you comment this out, you'll probably be able to sculpt normally but your normals will be wrong in non-optimal drawing. The change is very subtle, you might miss the error unless you expect it, it should be apparent there. Try GLSL mode it should appear there.

@Antony Riakiotakis (psy-fi), to fix the crash, committed check that the derived mesh isn't using a constructive modifier (it doesn't make any sense to copy normals between the arrays in that case - its random luck if their aligned or not).

Regarding your last comments, I think I see what you mean in principle, but I still can't see any shading difference with GLSL+Sculpt (matcaps / flat / smooth / with/without autosmooth ...).

When you have a modifier applied, the modifier stack will calculate the derived-mesh normals.

In the file from this report, for example - its calling CDDM_calc_normals_mapping_ex,
in fact with a deform modifier, with and without autosmooth, its calculating normals in sculpt mode, see call stack: P314.

(IIRC auto-smooth normals were meant to be disabled while sculpting).

Re-opening this report, since there are some unresolved issues with normal calculation, (even though the crash is fixed).

@Antony Riakiotakis (psy-fi), would you be able to show a test-case (simple blend file) that requires cdDM_update_normals_from_pbvh to run?

Sergey Sharybin (sergey) closed this task as Resolved.Jan 15 2016, 12:32 PM

Remaining confusion should now be fixed in rB7776f03.