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

Raul Fernandez Hernandez · 2024-03-13T00:02:03+01:00

2024-03-13 00:02:03 +01:00

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

Workaround to eliminate the crash and no loss of functionality
has been detected compared to before.

Fix for Crash after undo "Mask slice to new object" #87243 Workaround to eliminate the crash and no loss of functionality has been detected compared to before.

BeforeBug.gif

929 KiB

AfterBug.gif

782 KiB

Raul Fernandez Hernandez added 32 commits 2024-03-13 00:02:04 +01:00

workaround for crash 3004997dae

Fix #117798 : "Nothing to Bake" when pose object is not selected f67ac0ffb5

Use `context.pose_object` when baking a pose, on on top of the objects
in `context.selected_editable_objects`. When in pose mode, it's expected
that the pose of the active will be baked, regardless of whether the
pose object is selected itself.

Pull Request: #119317

Build: Upgrade OSL to 1.13.7, OIDN to 2.2.1 on macOS 879c4024e3

Fix: handle cyclic links when detecting dangling reroutes 1008348d91

Fix of error from 95810eec8f

Pull Request: #119335

Fix #110562 : Crash animating node property affecting socket visibility fd1978b6fa

For various reasons, the animation system can't properly update the node tree
so that the socket availability caused by changing node enum properties
propagates completely. So animating node properties that affect
socket visibility to change isn't possible without issues like crashes.
Unfortunately that wasn't disallowed before. In this commit there is
a balance of disabling animation on sockets that could reasonably expected
to affect socket visibility, and minimizing breaking changes.

Pull Request: #119221

Fix #90999 : crash when converting volume grid with high indices to dense volumes 3e07ff7840

There seems to be an integer overflow in OpenVDB code. For now just avoid rendering
the volume when the indices are very large, which is an extreme case anyway.

Pull Request: #119279

Fix #113170 : node editor background color affects node panel color 1180f82a2e

The offset `-15` that's passed to `UI_GetThemeColorShade4fv` is chosen so that the color
stays almost the same in the default theme.

Pull Request: #119283

Fix #46809 : History gets rearranged in Python console 6c0e1e0461

Lines in the history never change, each input line is added to the
end (unless it is a duplicate of the last item).
A new history_index member keeps track of where the up/down arrows have
moved in the history.

Preserved the previous down-arrow behavior of going to the item after
the one that was copied, this is useful in Python for re-entering a
multi-line block.

Ref !119356

Fix missing call to PyObject_GC_UnTrack WITH_PYTHON_SECURITY 730f87c57e

Untrack all newly created objects, although this code-path only runs
when creating the type fails - which is an exceptional case.

Fix error freeing bpy_struct sub-classes WITH_PYTHON_SECURITY enabled 546880b7d7

Python's behavior changed since this feature was added causing the
object to be tracked when freed by Python's subtype_dealloc even
if Blender has not tracked the data. Detect this case and untrack
the object before freeing.

Unbreak build with USE_EVENT_BACKGROUND_THREAD disabled 787c69aab8

I18N: Updated UI translations from git/weblate (efe372a7b46cf). aa665624a7

Cleanup: format 4d8f61bfb5

Fix #115185 : Batch Rename Action Clips error 435b211a9d

Account for actions themselves being selected in the outliner as well
as ID-types that don't have any animation data.

Fix #119309 : Face Set from Visible crash with multires 430cf92f5f

Fix is same as 18912561b5 / 72d324bd81

Pull Request: #119311

Fix #114438 : wrong undo after running node tool in mesh edit mode 4b368a6626

I think the issue is that the run-node-group operator overwrites the data stored in the
`Mesh` while mesh edit mode operations typically only change the `BMEditMesh`. It
seems like that causes issues because the mesh edit mode undo stack does not
keep track of changes to the `Mesh`. When hitting undo, Blender assumes that the
`Mesh` stored in the object has not changed and therefore it does not have to be
read from the undo step again.

The preliminary fix implemented here is to just not change the `Mesh` but only the
`BMEditMesh` like any other edit mode operator. This seems to solve the issue.
I haven't quite figured out yet how to tell the undo system that the `Mesh` has to be
loaded from the undo step when undoing out of mesh edit mode. Doing that might
provide a better solution.

Pull Request: #119015

Fix #119364 : capturing attributes on output object instances lose object visibility settings 4d31697e0d

The regression happened because object instances were turned into normal geometry
instances which don't have object-level visibility settings. Long term, this may not be
something we can support, but it's also not something we should break unnecessarily
and accidentally.

Pull Request: #119370

Fix #115313 : GPencil brush direction is not kept 45756a59de

The core of the issue was that `sculpt_flag` was used by three different enums (`eGP_Sculpt_Flag`, `eGP_Sculpt_Mode_Flag`, and `eBrushFlags`). This resulted in the flag getting overriden because `ENUM_OPERATORS` expected the maximum value of `eGP_Sculpt_Flag` to be `(1 << 3)` which it wasn't.

The `sculpt_flag` was exposed through python as `"direction"`.
In the UI this meant that it was effectively used as `brush.direction`. This fix replaces `brush.gpencil_settings.direction` with `brush.direction`.
It also makes sure `sculpt_flag` is only ever used with values from `eGP_Sculpt_Flag`.

Pull Request: #119373

Fix #118905 : Geometry Nodes: GP3 component and face corner in separate geometry node 948e5fb3c1

Pull Request: #118916

Fix #119021 : Crash when adding GP object 5cb5f0e8db

Added a null check for RE_GetRenderLayer() which could possibly return
null and then stored in `render_layer`, in order to prevent members of
`render_layer` being accessed in RE_pass_find_by_name(), and thus solved
the segmentation fault.

Pull Request: #119200

Fix #91256 , #112646 : IDProperty UI data lost when changing type 39a1e71c4f

With some data-type conversions we can do a best-effort conversion of
UI data like default values and min and max to the new data type.
This can help to make Python scripts simpler and to avoid bugs like
#105965.

Pull Request: #106161

Fix #119361 : Sculpt face set draw broken with multires 3507ac8f9e

`PBVHVertexIter::g` is the grid index within the node, not the the global grid index.

Fix #119239 : Use attribute names after free 0c74ea2ea9

Fix of error introduced in c31718649d. Attribute names will
be freed on domain resizing. This mean, ref-names which is attribute
ids is will be invalid. To avoid this, make sure names will be gathered
only after resize. To avoid unnecessary topology map computation before
mesh resize, check if attributes on required domain exists, instead
of gathering them and check if span is not empty.

Pull Request: #119242

Fix #116262 : Modal knife tool can crash if used on hidden object 873d94ad73

This could happen if objects were hidden after going to editmode and
knife was used on them.

In that case, raycasting would (rightfully) fail, but there is a
fallback in place using the back-buffer selection method (in which the
hidden object is still present). So a face would be found, but this
makes all following code confusing/wrong since we are working with
coordinates / faces under the assumption there would be a valid/
corresponding object to it in `KnifeTool_OpData` > `objects` -- which is
not the case...

So to resolve, just check if the object is visible before calling
`EDBM_face_find_nearest` in knife code.

Alternativeliy, we could also add a check for Base viewport visibility
to all the bmesh `find_nearest` functions (a bit hesitant though since i
am not sure this would be desired in all cases)

Pull Request: #119383

Fix #117603 : Auto smooth replacement versioning skipped for link/append c41c4a91c8

Unfortunately the only versioning code that can "properly" add data-
blocks is not run when linking or appending (`do_versions_after_setup`
has details). The versioning has to be done manually for this case.
Fortunately that is simple since the versioning function already just
operators on a main database.

Fix: update comparator for std::sort for NamedAttribute bdd967ba81

This PR applies a fix for the comparator being passed into std::sort to
avoid potential Mac / libc++ issues as seen in #119189

Pull Request: #119385

Fix #119350 : shifting text overlay text f47e736215

The issue was that the object location was taken into account twice using
`object_to_world` and `ob->loc`.

Pull Request: #119378

Fix: Slider not respecting bounds when snapping to increments 3d38e04185

The issue was that the clamping to the bounds happened before the rounding to the increment step.
In the reported case this led to a divide by 0 error.
The fix is to do the increment first, then the clamp to bounds.

This was reported by Raymond Luc on #117287

Pull Request: #119367

Fix #119307 : Anchored multires mask brush broken 022c5d58bf

I ran into this issue a while ago too, where the vertex iteration
macro didn't process all of the vertices. I didn't figure it out yet,
but it makes more sense in the context of the surrounding code
to specialize this anyway.

Fix #119396 : Add toggle for texture paint mask icon 0745a71532

This PR adjusts the behavior of the Texture Paint mask icon to behave
similarly to the automasking icon in Sculpt mode. This behavior was
missed in the original task: #102585

Pull Request: #119399

cleanup 020a3322f2

Merge branch 'blender-v4.1-release' into Fix-87243-Crash-after-undo-Mask-slice-to-new-object 0ef6e0f039

Raul Fernandez Hernandez changed title from ~~Fix 87243 Crash after undo "Mask slice to new object"~~ to WIP: Fix 87243 Crash after undo "Mask slice to new object"

2024-03-13 00:08:50 +01:00

Raul Fernandez Hernandez added the

Module

Sculpt, Paint & Texture

label 2024-03-13 00:14:55 +01:00

Raul Fernandez Hernandez requested review from Hans Goudey 2024-03-13 00:15:24 +01:00

Raul Fernandez Hernandez requested review from Sergey Sharybin 2024-03-13 00:15:37 +01:00

Raul Fernandez Hernandez requested review from Julien Kaspar 2024-03-13 00:16:19 +01:00

Raul Fernandez Hernandez changed title from ~~WIP: Fix 87243 Crash after undo "Mask slice to new object"~~ to Fix 87243 Crash after undo "Mask slice to new object"

2024-03-13 02:27:31 +01:00

Raul Fernandez Hernandez changed title from ~~Fix 87243 Crash after undo "Mask slice to new object"~~ to Fix 87243: Crash after undo "Mask slice to new object"

2024-03-13 02:37:54 +01:00

Sergey Sharybin changed title from ~~Fix 87243: Crash after undo "Mask slice to new object"~~ to Fix #87243: Crash after undo "Mask slice to new object"

2024-03-13 12:18:18 +01:00

Sergey Sharybin commented

2024-03-13 12:28:40 +01:00

The list of commits in this PR seems a bit strange. Did you create your branch from main and targeted it to the 4.1 branch?

For the content of the patch, it might be better to handle the null-pointer checks separately from the fix. Adding them makes discussion about the actually changed logic more tricky, and opens discussion of leaving undo stack in a potentially inconsistent state due to early returns.

And for the fix, would it help moving creation of the new object prior to the sculpt_paint::undo::geometry_begin ?

The list of commits in this PR seems a bit strange. Did you create your branch from main and targeted it to the 4.1 branch? For the content of the patch, it might be better to handle the null-pointer checks separately from the fix. Adding them makes discussion about the actually changed logic more tricky, and opens discussion of leaving undo stack in a potentially inconsistent state due to early returns. And for the fix, would it help moving creation of the new object prior to the `sculpt_paint::undo::geometry_begin` ?

Hans Goudey commented

2024-03-13 14:57:10 +01:00

Agreed with Sergey's comment. For fixes, the change should be as minimal as possible to correct the bug. Also, the commit should mention why a "workaround" is pursued here instead of a proper fix to the undo system where the crash is happening. At least the context is important to explain why this approach is taken.

Raul Fernandez Hernandez commented

2024-03-13 16:51:14 +01:00

Thanks for the reviews and feedback. @Sergey it was probably a branch from main or a merge with release-4.1 that went sideways,
for that I will cherry-pick the changes from this PR to a new one.
I've tried also moving the creation of the new object prior to the sculpt_paint::undo::geometry_begin and the bug persist.
Creating that new object in the middle of sculpt mode mess up with the undo.

Thanks for the reviews and feedback. @Sergey it was probably a branch from main or a merge with release-4.1 that went sideways, for that I will cherry-pick the changes from this PR to a new one. I've tried also moving the creation of the new object prior to the sculpt_paint::undo::geometry_begin and the bug persist. Creating that new object in the middle of sculpt mode mess up with the undo.

Raul Fernandez Hernandez changed title from ~~Fix #87243: Crash after undo "Mask slice to new object"~~ to WIP: Fix #87243: Crash after undo "Mask slice to new object"

2024-03-13 16:51:37 +01:00

Raul Fernandez Hernandez added 1 commit 2024-03-13 17:07:23 +01:00

nit pick 81ea4d158b

Raul Fernandez Hernandez added 1 commit 2024-03-13 19:14:28 +01:00

minimize changes b364743edf

Hans Goudey requested changes 2024-03-13 21:08:00 +01:00

Hans Goudey left a comment

Looks simpler now, thanks.

I will cherry-pick the changes from this PR to a new one

Not sure it's worth making a new PR here. This will get squashed once it's committed anyway.

Looks simpler now, thanks. >I will cherry-pick the changes from this PR to a new one Not sure it's worth making a new PR here. This will get squashed once it's committed anyway.

👍 1

source/blender/editors/mesh/editmesh_mask_extract.cc

						
				@ -463,3 +463,3 @@

				  View3D *v3d = CTX_wm_view3d(C);

				  BKE_sculpt_mask_layers_ensure(nullptr, nullptr, ob, nullptr);

				  bool create_new_object = RNA_boolean_get(op->ptr, "new_object");

Hans Goudey commented

2024-03-13 20:59:21 +01:00

Could you remove the change moving the fetching of RNA properties fill_holes and mask_threshold from this PR? It's just more noise that makes the actual fix less obvious.

Could you remove the change moving the fetching of RNA properties `fill_holes` and `mask_threshold` from this PR? It's just more noise that makes the actual fix less obvious.

Raul Fernandez Hernandez commented

2024-03-13 21:18:50 +01:00

I don't think is noise , it simplifies the calling and gets reused in the 2 calls later, making them shorter so the
formatter does not have to split the function into several lines.

I don't think is noise , it simplifies the calling and gets reused in the 2 calls later, making them shorter so the formatter does not have to split the function into several lines.

Hans Goudey commented

2024-03-13 22:47:32 +01:00

Sure, as a cleanup it's great. But it's not a fix

source/blender/editors/mesh/editmesh_mask_extract.cc

						
				@ -469,2 +473,2 @@

				  if (ob->mode == OB_MODE_SCULPT) {

				  /* Fix for https://projects.blender.org/blender/blender/issues/87243

				   Undo crashes when new object is created in the middle of a sculpt */