Fix #109460: vertex weight slider not working correctly #109665

Jacques Lucke · 2023-07-03T19:13:03+02:00

Jacques Lucke commented

2023-07-03 19:13:03 +02:00

#109460 was caused by the introduction of implicit sharing for custom data layers (7eee378ecc). Due to implicit sharing, the vertex weight layer is now shared between the original and evaluated object in object/weight paint mode, but not in edit mode because of bmesh. For that reason, the original data has to be copied to be able to edit it, which causes its pointer to change. uiDefButF can't deal with the situation when the pointer changes on every redraw.

The solution is to just use a global value to "send" data between the uiBut and the stored vertex group weight.

#109460 was caused by the introduction of implicit sharing for custom data layers (7eee378eccc8f87e1330b9cfea2799928be9d657). Due to implicit sharing, the vertex weight layer is now shared between the original and evaluated object in object/weight paint mode, but not in edit mode because of bmesh. For that reason, the original data has to be copied to be able to edit it, which causes its pointer to change. `uiDefButF` can't deal with the situation when the pointer changes on every redraw. The solution is to just use a global value to "send" data between the `uiBut` and the stored vertex group weight.

Jacques Lucke added 2 commits 2023-07-03 19:13:14 +02:00

ef9f39a4b9 fix

b6df348e53 fix

Jacques Lucke requested review from Sergey Sharybin 2023-07-03 19:19:24 +02:00

Sergey Sharybin commented

2023-07-03 19:45:33 +02:00

Not sure there is a more clear way of doing it. It seems that some similar cases are constructing RNA property pointer. Not sure it can work here.

@JulianEisel or @brecht are more familiar with what's possible in the UI than me.

Not sure there is a more clear way of doing it. It seems that some similar cases are constructing RNA property pointer. Not sure it can work here. @JulianEisel or @brecht are more familiar with what's possible in the UI than me.

Brecht Van Lommel commented

2023-07-03 20:15:16 +02:00

For that reason, the original data has to be copied to be able to edit it, which causes its pointer to change.

Can you say where this copy happens?

> For that reason, the original data has to be copied to be able to edit it, which causes its pointer to change. Can you say where this copy happens?

Jacques Lucke commented

2023-07-03 20:19:44 +02:00

Didn't check in detail right now, but most likely happens in the call to ED_mesh_active_dvert_get_only(ob) in the same function I changed, which returns a non-const pointer to the vertex weight, so it needs a copy if the data is shared with evaluated data already.

Didn't check in detail right now, but most likely happens in the call to `ED_mesh_active_dvert_get_only(ob)` in the same function I changed, which returns a non-const pointer to the vertex weight, so it needs a copy if the data is shared with evaluated data already.

Brecht Van Lommel requested changes 2023-07-04 15:45:17 +02:00

Brecht Van Lommel left a comment

This seems like it could bite us in more places, but I also don't have a good general solution for it. We could add some unique identifier to buttons that is used to find a correspondence between buttons across redraws, but that's also quite obscure.

Rather than using a global, this should be stored in TransformProperties along with the various median property values that can be edited here. A global is not strictly correct if there are multiple scenes open at the same time.

If you haven't already, please check if there are more buttons in this file that could have the same problem.

This seems like it could bite us in more places, but I also don't have a good general solution for it. We could add some unique identifier to buttons that is used to find a correspondence between buttons across redraws, but that's also quite obscure. Rather than using a global, this should be stored in `TransformProperties` along with the various median property values that can be edited here. A global is not strictly correct if there are multiple scenes open at the same time. If you haven't already, please check if there are more buttons in this file that could have the same problem.

Jacques Lucke added 4 commits 2023-08-01 11:37:04 +02:00

48521aea83 Merge branch 'main' into fix-vertex-weight-slider

16ae9892f4 fix after merge

b229f15575 move property to TransformProperties

bc3e9a3909 rename

Jacques Lucke commented

2023-08-01 11:38:57 +02:00

We could add some unique identifier to buttons that is used to find a correspondence between buttons across redraws, but that's also quite obscure.

Don't need to do that now, but generally I think that would be the right idea. Other UI frameworks also require this in cases when the mapping between redraws can't happen automatically.

If you haven't already, please check if there are more buttons in this file that could have the same problem.

Looks like this was the only remaining one. All other calls to uiDefButF already pass a pointer to something in TransformProperties.

> We could add some unique identifier to buttons that is used to find a correspondence between buttons across redraws, but that's also quite obscure. Don't need to do that now, but generally I think that would be the right idea. Other UI frameworks also require this in cases when the mapping between redraws can't happen automatically. > If you haven't already, please check if there are more buttons in this file that could have the same problem. Looks like this was the only remaining one. All other calls to `uiDefButF` already pass a pointer to something in `TransformProperties`.

Jacques Lucke requested review from Brecht Van Lommel 2023-08-01 11:39:11 +02:00

Brecht Van Lommel approved these changes 2023-08-01 13:19:08 +02:00

Jacques Lucke merged commit cd74ba6413 into main

2023-08-01 13:24:30 +02:00

Jacques Lucke deleted branch fix-vertex-weight-slider

2023-08-01 13:24:32 +02:00

Jacques Lucke referenced this issue from a commit

2023-08-01 13:24:33 +02:00

Fix #109460: vertex weight slider not working correctly

Philipp Oeser referenced this issue from a commit

2023-08-09 18:37:49 +02:00

Fix #109460: vertex weight slider not working correctly

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #109460: vertex weight slider not working correctly #109665