Regression: Hair Particle Edit on model with Mirror Modifier not working properly #107677

New Issue

Closed

opened 2023-05-06 06:53:05 +02:00 by Mohamad Faisal Amir · 10 comments

Mohamad Faisal Amir commented

2023-05-06 06:53:05 +02:00

System Information
Operating system: Windows 10 Home. 100.19044 Build 19044
Processor: Intel Core i7-8750H
RAM: 16 GB
Graphics card: GTX 1060 6GB

Blender Version
Broken: version: 3.1.0, branch: master, commit date: 2022-03-08 18:16, hash: rBc77597cd0e15
Worked: version: 3.1.0 Beta, branch: master, commit date: 2022-02-20 22:00, hash: rB1a0a22f95a22

Caused by 2fb31f34af

Short description of error
Add hair Particle on Particle Edit mode not working properly on model with Mirror Modifier. The mirrored particle root appear offset too much from expected.
Please check the attached Video and Blend File.

Exact steps for others to reproduce the error

Add any object cube or sphere.
Slice in half.
Add Modifier: Mirror.
Add Particle. type: hair.
a. Set Emission > Number = 0 to add manually later.
b. Set Emission > Source > Check USE MODIFIER STACK.
In 3D Viewport change into Particle Edit mode.
Use Add tool.
a. On the option panel, check Mirror X.
Start add hair particle.

The mirrored particle will appear on the wrong place.
I've tested this on Blender 3.4, 3.2, 3.1 and this behavior also appear.
But it just work fine on Blender 3.0.1 just like on the video

**System Information** Operating system: Windows 10 Home. 100.19044 Build 19044 Processor: Intel Core i7-8750H RAM: 16 GB Graphics card: GTX 1060 6GB **Blender Version** Broken: version: 3.1.0, branch: master, commit date: 2022-03-08 18:16, hash: `rBc77597cd0e15` Worked: version: 3.1.0 Beta, branch: master, commit date: 2022-02-20 22:00, hash: `rB1a0a22f95a22` Caused by 2fb31f34af84c39116742e20a12222099cd0ff9a **Short description of error** Add hair Particle on Particle Edit mode not working properly on model with Mirror Modifier. The mirrored particle root appear offset too much from expected. Please check the attached Video and Blend File. **Exact steps for others to reproduce the error** 1. Add any object cube or sphere. 2. Slice in half. 3. Add Modifier: Mirror. 4. Add Particle. type: hair. a. Set Emission > Number = 0 to add manually later. b. Set Emission > Source > Check USE MODIFIER STACK. 5. In 3D Viewport change into Particle Edit mode. 6. Use Add tool. a. On the option panel, check Mirror X. 7. Start add hair particle. The mirrored particle will appear on the wrong place. I've tested this on Blender 3.4, 3.2, 3.1 and this behavior also appear. But it just work fine on Blender 3.0.1 just like on the video

mirror particle Blender 3.5.1 test.blend

826 KiB

01 Blender 3.5.1 issue.PNG

378 KiB

02 Blender 3.0.1.PNG

322 KiB

01 Blender 3.5.1.mp4

5.1 MiB

02 Blender 3.0.1.mp4

4.1 MiB

01 Blender 3.5.1 issue.PNG

02 Blender 3.0.1.PNG

Mohamad Faisal Amir added the

labels 2023-05-06 06:53:06 +02:00

Iliya Katushenock added the

Nodes & Physics

Sculpt, Paint & Texture

labels 2023-05-06 12:56:59 +02:00

Iliya Katushenock changed title from ~~Hair Particle Edit on model with Mirror Modifier not working properly~~ to Regression: Hair Particle Edit on model with Mirror Modifier not working properly

2023-05-06 12:57:06 +02:00

Iliya Katushenock added

Nodes & Physics

and removed

Nodes & Physics

labels 2023-05-06 16:26:37 +02:00

Iliya Katushenock added

and removed

labels 2023-05-06 16:34:17 +02:00

Iliya Katushenock commented

2023-05-06 16:34:52 +02:00

Member

Most likely this will not be fixed, since soon the old particles will be replaced with curves.

Most likely this will not be fixed, since soon the old particles will be replaced with curves.

Philipp Oeser commented

2023-05-08 09:41:42 +02:00

Member

Will still check on this

Will still check on this

Philipp Oeser commented

2023-05-08 10:47:40 +02:00

Member

Caused by 2fb31f34af

We are now calling BKE_mesh_orco_verts_transform not only from psys_face_mat and psys_mat_hair_to_orco, but now also from psys_particle_on_dm > psys_interpolate_face, so we get double transforms with mirror.

But havent thought about this long enough to decide where to call this conditionally (or remove).

Proof of concept though: removing from psys_mat_hair_to_orco seems to work in simple tests:

diff --git a/source/blender/blenkernel/intern/particle.cc b/source/blender/blenkernel/intern/particle.cc
index 0320f98e349..bc5c58f51fe 100644
--- a/source/blender/blenkernel/intern/particle.cc
+++ b/source/blender/blenkernel/intern/particle.cc
@@ -3957,10 +3957,6 @@ void psys_mat_hair_to_orco(
   psys_particle_on_dm(
       mesh, from, pa->num, pa->num_dmcache, pa->fuv, pa->foffset, vec, 0, 0, 0, orco);
 
-  /* see psys_face_mat for why this function is called */
-  if (CustomData_get_layer(&mesh->vdata, CD_ORIGINDEX)) {
-    BKE_mesh_orco_verts_transform(static_cast<Mesh *>(ob->data), &orco, 1, 1);
-  }
   copy_v3_v3(hairmat[3], orco);
 }

CC @JacquesLucke
CC @Sergey

Caused by 2fb31f34af84c39116742e20a12222099cd0ff9a We are now calling `BKE_mesh_orco_verts_transform` not only from `psys_face_mat` and `psys_mat_hair_to_orco`, but now also from `psys_particle_on_dm` > `psys_interpolate_face`, so we get double transforms with mirror. But havent thought about this long enough to decide where to call this conditionally (or remove). Proof of concept though: removing from `psys_mat_hair_to_orco` seems to work in simple tests: ```Diff diff --git a/source/blender/blenkernel/intern/particle.cc b/source/blender/blenkernel/intern/particle.cc index 0320f98e349..bc5c58f51fe 100644 --- a/source/blender/blenkernel/intern/particle.cc +++ b/source/blender/blenkernel/intern/particle.cc @@ -3957,10 +3957,6 @@ void psys_mat_hair_to_orco( psys_particle_on_dm( mesh, from, pa->num, pa->num_dmcache, pa->fuv, pa->foffset, vec, 0, 0, 0, orco); - /* see psys_face_mat for why this function is called */ - if (CustomData_get_layer(&mesh->vdata, CD_ORIGINDEX)) { - BKE_mesh_orco_verts_transform(static_cast<Mesh *>(ob->data), &orco, 1, 1); - } copy_v3_v3(hairmat[3], orco); } ``` CC @JacquesLucke CC @Sergey

Philipp Oeser removed the

Sculpt, Paint & Texture

label 2023-05-08 10:48:09 +02:00

Jacques Lucke commented

2023-05-08 11:43:29 +02:00

Member

The proposed fix of removing these lines sounds reasonable to me. Applying BKE_mesh_orco_verts_transform on the same variable twice does not make sense. I don't see a reason for why this should ever be called conditionally right now.

The proposed fix of removing these lines sounds reasonable to me. Applying `BKE_mesh_orco_verts_transform` on the same variable twice does not make sense. I don't see a reason for why this should ever be called conditionally right now.

Philipp Oeser commented

2023-05-08 11:52:47 +02:00

Member

The thing I was wondering about is why we even call it multiple times (even with the patch it gets called twice afaict)

The thing I was wondering about is why we even call it multiple times (even with the patch it gets called twice afaict)

Jacques Lucke commented

2023-05-08 11:56:25 +02:00

Member

Not sure which two calls you mean right now but note that BKE_mesh_orco_verts_transform has an "invert" parameter. So maybe it's called twice to convert the data back and forth?

Not sure which two calls you mean right now but note that `BKE_mesh_orco_verts_transform` has an "invert" parameter. So maybe it's called twice to convert the data back and forth?

Philipp Oeser commented

2023-05-08 12:03:24 +02:00

Member

Not sure which two calls you mean right now but note that BKE_mesh_orco_verts_transform has an "invert" parameter. So maybe it's called twice to convert the data back and forth?

psys_face_mat and psys_particle_on_dm > psys_interpolate_face both call it inverted, so might be it?

> Not sure which two calls you mean right now but note that `BKE_mesh_orco_verts_transform` has an "invert" parameter. So maybe it's called twice to convert the data back and forth? `psys_face_mat` and `psys_particle_on_dm` > `psys_interpolate_face` both call it inverted, so might be it?

Jacques Lucke commented

2023-05-08 12:12:04 +02:00

Member

From what I can tell right now, it always gets vectors from orcodata and then calls BKE_mesh_orco_verts_transform (with invert=true). The fact that it is called twice does not matter, because it's not the case that the output of the first call is passed into the second call, I think.

From what I can tell right now, it always gets vectors from `orcodata` and then calls `BKE_mesh_orco_verts_transform` (with `invert=true`). The fact that it is called twice does not matter, because it's not the case that the output of the first call is passed into the second call, I think.

Philipp Oeser commented

2023-05-09 10:40:08 +02:00

Member

The fact that it is called twice does not matter, because it's not the case that the output of the first call is passed into the second call, I think.

Hm, why does the third call [which the proposed patch removes] cause a double transform then?

> The fact that it is called twice does not matter, because it's not the case that the output of the first call is passed into the second call, I think. Hm, why does the third call [which the proposed patch removes] cause a double transform then?

Jacques Lucke commented

2023-05-09 12:11:51 +02:00

Member

Hm, why does the third call [which the proposed patch removes] cause a double transform then?

Because both calls to BKE_mesh_orco_verts_transform get the same orco variable passed in afaik.

> Hm, why does the third call [which the proposed patch removes] cause a double transform then? Because both calls to `BKE_mesh_orco_verts_transform` get the same `orco` variable passed in afaik.

Philipp Oeser referenced a pull request that will close this issue

2023-05-10 09:37:08 +02:00

Fix #107677: Hair particle edit wrong on object with mirror modifier #107804

Philipp Oeser closed this issue

2023-05-10 15:16:27 +02:00

Philipp Oeser referenced this issue from a commit

2023-05-10 15:16:33 +02:00

Fix #107677: Hair particle edit wrong on object with mirror modifier

Blender Bot added

and removed

labels 2023-05-10 15:16:53 +02:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project (Legacy)

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#107677

No description provided.