Fix #87243: Crash after undo "Mask slice to new object" #119443

Raul Fernandez Hernandez · 2024-03-13T21:25:33+01:00

2024-03-13 21:25:33 +01:00

Fix for a crash after undo "Mask slice to new object"

When creating a new mesh object in the mask slice operator in
sculpt mode it caused a crash when undo to the initial undo step.
Moreover saving the sculpt undo state seems not necessary in
this function since undo/redo doesn't really work reliably for this
operator from the start, but there is nothing can be done about it
without systemic changes.

Note: this PR replaces #119401

Fix for a crash after undo "Mask slice to new object" When creating a new mesh object in the mask slice operator in sculpt mode it caused a crash when undo to the initial undo step. Moreover saving the sculpt undo state seems not necessary in this function since undo/redo doesn't really work reliably for this operator from the start, but there is nothing can be done about it without systemic changes. Note: this PR replaces #119401

AfterBug.gif

782 KiB

BeforeBug.gif

929 KiB

Raul Fernandez Hernandez added 2 commits 2024-03-13 21:25:57 +01:00

f20e48c814 fix

c335802c53 nit

Raul Fernandez Hernandez referenced this pull request

2024-03-13 21:27:34 +01:00

WIP: Fix #87243: Crash after undo "Mask slice to new object" #119401

Raul Fernandez Hernandez added the

Module

Sculpt, Paint & Texture

label 2024-03-13 21:28:09 +01:00

Sergey Sharybin was assigned by Raul Fernandez Hernandez

2024-03-13 21:29:14 +01:00

Hans Goudey was assigned by Raul Fernandez Hernandez

2024-03-13 21:29:14 +01:00

Raul Fernandez Hernandez requested review from Sergey Sharybin 2024-03-13 21:32:19 +01:00

Raul Fernandez Hernandez requested review from Hans Goudey 2024-03-13 21:32:20 +01:00

Sergey Sharybin was unassigned by Raul Fernandez Hernandez

2024-03-13 21:32:37 +01:00

Hans Goudey was unassigned by Raul Fernandez Hernandez

2024-03-13 21:32:37 +01:00

Raul Fernandez Hernandez self-assigned this 2024-03-13 21:32:43 +01:00

Sergey Sharybin commented

2024-03-14 11:25:15 +01:00

Moreover saving the sculpt undo state seems not necessary in this function since undo/redo works anyways.

It does not seem to be working: the mask attribute seems to be lost, and the initial geometry of the object is not preserved, with some notes:

Testing this is a bit annoying, since the operator changes the active object, which is not restored by the undo. I think this is because when operator run a local sculpt undo system is used, which does not cover changes of active object.
This behavior does not seem to be changed comparing to the current main branch (tested by undoing to the state proir to which Blender crashes)

I've tried to find a safe solution which does not require a huge amount of system redesign and I couldn't have found one. The operator is kind of a violation of a design of local modes which are not supposed to be doing global changes (such as adding new objects from sculpt/edit modes).

To is is probably fine to go with the suggested change, but we need to change the way it is adertized from "undo/redo works anyways" to "undo/redo doesn't really work reliably, but there is nothing can be done about it without more global changes" type of a thing.

> Moreover saving the sculpt undo state seems not necessary in this function since undo/redo works anyways. It does not seem to be working: the mask attribute seems to be lost, and the initial geometry of the object is not preserved, with some notes: - Testing this is a bit annoying, since the operator changes the active object, which is not restored by the undo. I think this is because when operator run a local sculpt undo system is used, which does not cover changes of active object. - This behavior does not seem to be changed comparing to the current main branch (tested by undoing to the state proir to which Blender crashes) I've tried to find a safe solution which does not require a huge amount of system redesign and I couldn't have found one. The operator is kind of a violation of a design of local modes which are not supposed to be doing global changes (such as adding new objects from sculpt/edit modes). To is is probably fine to go with the suggested change, but we need to change the way it is adertized from "undo/redo works anyways" to "undo/redo doesn't really work reliably, but there is nothing can be done about it without more global changes" type of a thing.

👍 1

Raul Fernandez Hernandez commented

2024-03-14 16:17:53 +01:00

Moreover saving the sculpt undo state seems not necessary in this function since undo/redo works anyways.

It does not seem to be working: the mask attribute seems to be lost, and the initial geometry of the object is not preserved, with some notes:

Testing this is a bit annoying, since the operator changes the active object, which is not restored by the undo. I think this is because when operator run a local sculpt undo system is used, which does not cover changes of active object.

This behavior does not seem to be changed comparing to the current main branch (tested by undoing to the state proir to which Blender crashes)

I've tried to find a safe solution which does not require a huge amount of system redesign and I couldn't have found one. The operator is kind of a violation of a design of local modes which are not supposed to be doing global changes (such as adding new objects from sculpt/edit modes).

To is is probably fine to go with the suggested change, but we need to change the way it is adertized from "undo/redo works anyways" to "undo/redo doesn't really work reliably, but there is nothing can be done about it without more global changes" type of a thing.

Thanks! Agree, that's what I found too, the operator from the beginning messes up the undo/redo with the change of the active object, the change from sculpt mode to object mode, etc that will require a deeper system refactor.

> > Moreover saving the sculpt undo state seems not necessary in this function since undo/redo works anyways. > > It does not seem to be working: the mask attribute seems to be lost, and the initial geometry of the object is not preserved, with some notes: > - Testing this is a bit annoying, since the operator changes the active object, which is not restored by the undo. I think this is because when operator run a local sculpt undo system is used, which does not cover changes of active object. > - This behavior does not seem to be changed comparing to the current main branch (tested by undoing to the state proir to which Blender crashes) > > I've tried to find a safe solution which does not require a huge amount of system redesign and I couldn't have found one. The operator is kind of a violation of a design of local modes which are not supposed to be doing global changes (such as adding new objects from sculpt/edit modes). > > To is is probably fine to go with the suggested change, but we need to change the way it is adertized from "undo/redo works anyways" to "undo/redo doesn't really work reliably, but there is nothing can be done about it without more global changes" type of a thing. Thanks! Agree, that's what I found too, the operator from the beginning messes up the undo/redo with the change of the active object, the change from sculpt mode to object mode, etc that will require a deeper system refactor.

Raul Fernandez Hernandez referenced this issue from a commit

2024-03-14 17:02:01 +01:00

Fix #87243: Crash after undo "Mask slice to new object"

Raul Fernandez Hernandez merged commit 2e7395e07b into blender-v4.1-release

2024-03-14 17:02:11 +01:00

Raul Fernandez Hernandez deleted branch mask-slice-fix

2024-03-14 17:02:17 +01:00

Raul Fernandez Hernandez referenced this issue from a commit

2024-03-14 17:10:37 +01:00

Fix #87243: Crash after undo "Mask slice to new object"

Raul Fernandez Hernandez referenced this pull request

2024-03-18 21:53:05 +01:00