Regression: "Copy Location" constraint doesn't work if the control object is a vertex and we're in edit mode. #99141

New Issue

Tsvetelin Lyubenov · 2022-06-24T17:21:00+02:00

2022-06-24 17:21:00 +02:00

System Information
Operating system: Windows-10-10.0.19044-SP0 64 Bits
Graphics card: GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.51

Blender Version
Broken: 3.1.0, branch: master, commit date: 2022-03-08 18:16, hash: c77597cd0e and later.
Worked: version: 3.0.0, branch: master, commit date: 2021-12-02 18:35, hash: f1cca30557

So this first started crashing with cfa53e0fbe
The crash then vanished in 0f89bcdbeb, leaving the editmode failure in place.

Short description of error
"Copy Location" constraint doesn't work if the control object is a vertex(vertex group) and we're in edit mode.

Exact steps for others to reproduce the error

Open a new scene and delete the Cube, the Light and the Camera.
Create a simple Plane object.
Enter Edit Mode and select one vertex.
Create a vertex group, assign the selected vertex to it and go back to Object mode.
Create an Empty.
Add a "Copy Location" constraint to the Empty. Choose the Plane as target and specify as vertex group the one created in step 4.
Enter Edit Mode for the Plane and move the control vertex assigned to the vertex group in step 4.

The Empty should follow the controlling vertex but it doesn't in 3.1.0 and later builds.

I'm attaching the setup for convenience:
CopyLocation_Bug.blend

**System Information** Operating system: Windows-10-10.0.19044-SP0 64 Bits Graphics card: GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.51 **Blender Version** Broken: 3.1.0, branch: master, commit date: 2022-03-08 18:16, hash: `c77597cd0e` and later. Worked: version: 3.0.0, branch: master, commit date: 2021-12-02 18:35, hash: `f1cca30557` So this first started crashing with cfa53e0fbe The crash then vanished in 0f89bcdbeb, leaving the editmode failure in place. **Short description of error** "Copy Location" constraint doesn't work if the control object is a vertex(vertex group) and we're in edit mode. **Exact steps for others to reproduce the error** 1. Open a new scene and delete the Cube, the Light and the Camera. 2. Create a simple Plane object. 3. Enter Edit Mode and select one vertex. 4. Create a vertex group, assign the selected vertex to it and go back to Object mode. 5. Create an Empty. 6. Add a "Copy Location" constraint to the Empty. Choose the Plane as target and specify as vertex group the one created in step 4. 7. Enter Edit Mode for the Plane and move the control vertex assigned to the vertex group in step 4. The Empty should follow the controlling vertex but it doesn't in 3.1.0 and later builds. I'm attaching the setup for convenience: [CopyLocation_Bug.blend](https://archive.blender.org/developer/F13225061/CopyLocation_Bug.blend)

Tsvetelin Lyubenov commented

2022-06-24 17:21:00 +02:00

Added subscriber: @rendetto

Richard Antalik commented

2022-06-24 20:19:02 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2022-06-29 22:13:22 +02:00

Added subscribers: @HooglyBoogly, @Sergey, @lichtwerk

Philipp Oeser commented

2022-06-29 22:13:22 +02:00

So this first started crashing with cfa53e0fbe

The crash then vanished in 0f89bcdbeb, leaving the editmode failure in place.

CC @HooglyBoogly
CC @Sergey

So this first started crashing with cfa53e0fbe The crash then vanished in 0f89bcdbeb, leaving the editmode failure in place. CC @HooglyBoogly CC @Sergey

Philipp Oeser changed title from ~~"Copy Location" constraint doesn't work if the control object is a vertex and we're in edit mode.~~ to Regression: "Copy Location" constraint doesn't work if the control object is a vertex and we're in edit mode.

2022-06-29 22:13:47 +02:00

Hans Goudey commented

2022-06-29 23:06:22 +02:00

It looks like the constraint is attempting to access mesh normals on a mesh with wrapper type ME_WRAPPER_TYPE_BMESH.

It assumes that these two conditions are mutually exclusive, but apparently they are not:

  const Mesh *me_eval = BKE_object_get_evaluated_mesh(ob);
  BMEditMesh *em = BKE_editmesh_from_object(ob);

Actually, the use of BMEditMesh at all there seems quite suspect to me, particularly the comment that says "when not in EditMode, use the 'final' evaluated mesh, depsgraph ensures we build with CD_MDEFORMVERT layer"
Going into edit mode on the constraint's target object shouldn't change the effect of the constraint at all! The constraint should always use the evaluated geometry from another object.

So I'd propose removing the edit mesh handling entirely and using BKE_mesh_wrapper_ensure_mdata to ensure that we have proper Mesh data.

It looks like the constraint is attempting to access mesh normals on a mesh with wrapper type `ME_WRAPPER_TYPE_BMESH`. It assumes that these two conditions are mutually exclusive, but apparently they are not: ``` const Mesh *me_eval = BKE_object_get_evaluated_mesh(ob); BMEditMesh *em = BKE_editmesh_from_object(ob); ``` Actually, the use of `BMEditMesh` at all there seems quite suspect to me, particularly the comment that says "when not in EditMode, use the 'final' evaluated mesh, depsgraph ensures we build with CD_MDEFORMVERT layer" Going into edit mode on the constraint's **target** object shouldn't change the effect of the constraint at all! The constraint should always use the evaluated geometry from another object. So I'd propose removing the edit mesh handling entirely and using `BKE_mesh_wrapper_ensure_mdata` to ensure that we have proper `Mesh` data.

Sergey Sharybin commented

2022-06-30 09:55:47 +02:00

It assumes that these two conditions are mutually exclusive, but apparently they are not:
const Mesh *me_eval = BKE_object_get_evaluated_mesh(ob);
BMEditMesh *em = BKE_editmesh_from_object(ob);

That is quite unusual assumption. The proper way to think of it is that when there is an editmesh it is the source of truth.

Going into edit mode on the constraint's target object shouldn't change the effect of the constraint at all!

If the modifier is configured to be disabled in edit mode the difference in constrain behavior is inevitable.
The current design values performance of edit mode over complete correctness of cases when an edit object is sued as a target for something.

So I'd propose removing the edit mesh handling entirely and using BKE_mesh_wrapper_ensure_mdata to ensure that we have proper Mesh data.

I do not think this is a correct thing to do. You'll loose performance and increase memory usage by doing so.

To me the proper solution always seemed to be to make it so the decision about source of truth are hidden behind API of some sort. It might involve making it so the mesh wrapper API is more complete.

> It assumes that these two conditions are mutually exclusive, but apparently they are not: > const Mesh *me_eval = BKE_object_get_evaluated_mesh(ob); > BMEditMesh *em = BKE_editmesh_from_object(ob); That is quite unusual assumption. The proper way to think of it is that when there is an editmesh it is the source of truth. > Going into edit mode on the constraint's target object shouldn't change the effect of the constraint at all! If the modifier is configured to be disabled in edit mode the difference in constrain behavior is inevitable. The current design values performance of edit mode over complete correctness of cases when an edit object is sued as a target for something. > So I'd propose removing the edit mesh handling entirely and using `BKE_mesh_wrapper_ensure_mdata` to ensure that we have proper `Mesh` data. I do not think this is a correct thing to do. You'll loose performance and increase memory usage by doing so. To me the proper solution always seemed to be to make it so the decision about source of truth are hidden behind API of some sort. It might involve making it so the mesh wrapper API is more complete.

Hans Goudey commented

2022-09-06 06:34:09 +02:00

I still think the "dumb" behavior of just always using the evaluated mesh is preferable from the standpoint of making things predictable and generic, but so be it if the current design disagrees-- at least there is one!

To me the proper solution always seemed to be to make it so the decision about source of truth are hidden behind API of some sort. It might involve making it so the mesh wrapper API is more complete.

Personally I'm skeptical of changes that put BMesh and Mesh behind the same API, unless they only go in the direction of providing SoA access for BMesh data.

I still think the "dumb" behavior of just always using the evaluated mesh is preferable from the standpoint of making things predictable and generic, but so be it if the current design disagrees-- at least there is one! >To me the proper solution always seemed to be to make it so the decision about source of truth are hidden behind API of some sort. It might involve making it so the mesh wrapper API is more complete. Personally I'm skeptical of changes that put BMesh and Mesh behind the same API, unless they only go in the direction of providing SoA access for BMesh data.

blender-admin commented

2022-09-06 20:09:01 +02:00

This issue was referenced by 8004214356

This issue was referenced by 8004214356a4882606d92758f33954e5973fc591

blender-admin commented

2022-09-06 20:09:01 +02:00

This issue was referenced by 394c0b5ae4

This issue was referenced by 394c0b5ae4d9b28e8624ea831e20e011a687f71e

Hans Goudey commented

2022-09-06 20:19:41 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Hans Goudey closed this issue