Page MenuHome

Patch Proposed for T72490 - Collections: Exclude From View toggle causes segment violation
AbandonedPublic

Authored by Garry R. Osgood (grosgood) on Sun, Jan 5, 10:54 PM.

Details

Summary

This is a patch forwarded to address T72490. The specific trigger of this bug is a corner case somewhat removed from daily workflow. The user has to:

  1. Create a mesh, surface, curve, text or lattice object (a mesh object named "Mutt" in the example colselect.blend file serves).
  2. Put "Mutt" in a collection branched off the root Scene Collection. The example uses Mutt_Collection.
  3. Put "Mutt" in edit mode. Perform any number of edits on "Mutt", including none at all.
  4. Hide Mutt_Collection using the Exclude from View Layer checkbox. This is key: the user does NOT place the object, "Mutt," in any other mode. When being hidden via the collection, "Mutt" is still in edit mode. What is also key is the implicit "tab key action" which arises from hiding the collection. The viewport switches from edit mode to object mode, presumably because the object being edited is now hidden and nothing visible has been selected for editing. There is now something of a mismatch between the object, "Mutt", still in edit mode, and the viewport in object mode. "Mutt" is hidden via the collection it is in, however, so the mismatch is not obvious.
  5. Do any number of other things, including nothing at all.
  6. Unhide Mutt Collection. ""Mutt"" reappears, but not with the appearance of an object under edit. However, should one switch to the Data API view and look up "Mutt", it is marked as still being in edit mode.
  7. Click on "Mutt", perhaps to resume work on it.
  8. Blender crashes, as described in T72490. See also the included video clip.

Patch Notes
Functions DRW_draw_select_loop() or DRW_draw_render_loop_ex() orchestrate the execution of viewport refresh in response to mouse clicks, where one or the other inventories active viewport drawing engines, initializes them and ensures that caches and buffers are set up. They do not embody direct knowledge on how to do these things, but iterate over the active viewport engines and call function references on the engines themselves to perform actions given the exigencies of particular drawing engines.

Of concern here are the cache initialization and populate phases of the overlay engine, and the implementations provided by the overlay drawing engine, which is responsible for rendering various visual cues for the user: the grid plane, outlining of selected objects and the like. It provides implementations OVERLAY_cache_init() and OVERLAY_cache_populate() for the cache initialization and populate phases.

These two implementations coordinate their behavior in part through transient state, OVERLAY_PrivateData. Of interest is the enumeration field, eContextObjectMode ctx_mode. This drawing context mode enumeration encodes 'view operation' (object, edit...) on an object type (mesh, surface...). Thus drawing context mode CTX_MODE_EDIT_SURFACE reflects an edit mode context on a surface object. CTX_data_mode_enum_ex() determines this mode during the course of OVERLAY_engine_init().

Function OVERLAY_cache_init() uses the eContextObjectMode enumeration ctx_mode to choose an object-type-draw-context-operation specific initializer. Depending on these, OVERLAY_cache_init() chooses, say OVERLAY_edit_mesh_cache_init() for mesh objects being edited. Among other actions, these initializers populate OVERLAY_PrivateData for use by follow-on functions such as OVERLAY_edit_mesh_cache_populate(). There is potential coupling between function pairs such as OVERLAY_*_cache_init() and OVERLAY_edit_*_cache_populate() in that the populator can behave badly if data sets like OVERLAY_PrivateData have not been set up by the appropriate initializer, the root cause of this particular bug. In the example colselect.blend file, containing "Mutt", a mesh object, OVERLAY_cache_populate() chooses OVERLAY_edit_mesh_cache_populate() as if the prevailing drawing context mode was that "Mutt" was being edited (CTX_MODE_EDIT_MESH). But that is not the conclusion that OVERLAY_cache_init() draws. Finding a eContextObjectMode of CTX_MODE_OBJECT instead, it does not call OVERLAY_edit_mesh_cache_init() to initialize OVERLAY_PrivateData in the manner that OVERLAY_edit_mesh_cache_populate() expects, setting up the immediate circumstances of the crash. T72490 reviews these gory details.

The motivation of this patch is to harness eContextObjectMode ctx_mode to ensure that OVERLAY_cache_populate() chooses a cache populator appropriate for the prevailing drawing context mode. Absent this patch, OVERLAY_cache_populate() depends on BKE_object_is_in_editmode(ob) to set a flag (in_edit_mode). In common cases, the drawing context mode aligns with that of the object, but the peculiar action of Exclude from View Layer checkbox on a collection leaves the internal state of member objects undisturbed. BKE_object_is_in_editmode(ob) finds that "Mutt" has a mesh and is content to report to its caller that "Mutt" is in edit mode, and OVERLAY_cache_populate() is equally content to take that to mean that CTX_MODE_EDIT_MESH is the prevailing drawing mode context, without explicitly referencing ctx_mode. Pity.

This patch is an immediate and local approach to the bug. One could say it is a defensive measure against the lazy management of objects when collections are hidden. The more appropriate approach to this bug may lie in the direction of having the behavior of Exclude from View Layer to be more transparent to the rest of the application - given the viewport mode change that this toggle engenders, when a collection is toggled back into visibility, the contained objects should be mode-set appropriately.


colselect.blend

vidrpt_T72490.mkv

Diff Detail

Repository
rB Blender

Event Timeline

Hi @Garry R. Osgood (grosgood)

Thanks for the patch. As you mentioned this is a local fix. I have some minor code styling comments and the patch at all raises some questions.
I am not sure what the long term patch would look like and if this patch is complete (even as a local fix). It seems to be incorrect state in the Object.

source/blender/draw/engines/overlay/overlay_engine.c
231–233

code styling

  • tabs/spacing
  • normally we do if (pd->ctx_mode == CTX_MODE_EDIT_MESH) {

Cleanup per @Jeroen Bakker (jbakker) on Differential D6531: (1) tabs->spaces (2) <target field> == <query symbol>. Thank you for your style tips.

It seems to be incorrect state in the Object.

Yes. I think that 188: in_edit_mode = (ob->mode & OB_MODE_EDIT) && BKE_object_is_in_editmode(ob); resolves to true is not an anticipated circumstance with a prevailing pd->ctx_mode == CTX_MODE_OBJECT draw mode. This circumstance does not commonly arise, but toggling Exclude from View Layer does give rise to it. Why that is so is not entirely clear to me yet; I'm still quite junior in matters concerning the Blender object life cycle.

Generally, I don't advocate local and defensive conditionals at a level where one hopes for standard behavior. Wading through if-then-else thickets can be tiresome. I suggest this patch only in that it seals off a crash point of a most pernicious sort in that a user, in excluding from view a collection with objects in edit mode, sets up the circumstances for the crash perhaps minutes, hours or days before the trip event happens. Bringing the collection back into view and then selecting one of these objects in 'quasi-edit' mode brings about a crash long after the trigger circumstances have been put in play. Hence bug reports of the "Blender-crashes-I-don't-know-why" sort.

...and if this patch is complete (even as a local fix).

I'm content to await @Clément Foucault (fclem) observations on this.

Garry R. Osgood (grosgood) marked an inline comment as done.Sun, Jan 12, 5:13 PM
Garry R. Osgood (grosgood) added inline comments.
source/blender/draw/engines/overlay/overlay_engine.c
231–233

Done.

Geez... I should have commited this revision with my changes to close it and make you the author of the commit. Sorry for this... I hope you don't mind.

Final fix have been commited as rBe2724abc22d53f3a1f1552a6f40af3d47f7512e4

Garry R. Osgood (grosgood) marked an inline comment as done.Tue, Jan 14, 6:22 PM

@Clément Foucault (fclem)
Don't mind at all. `Twas a useful exercise for me. Thank you for all your good work.