rB0666ece2e2f9 (Cloth: collision improvements) breaks particle collisions with moving meshes #71620

New Issue

Kevin Buhr · 2019-11-16T04:24:49+01:00

Kevin Buhr commented

2019-11-16 04:24:49 +01:00

System Information
Operating system: Linux-4.15.0-70-generic-x86_64-with-debian-buster-sid 64 Bits
Graphics card: GeForce GTX 1080/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 440.26

Blender Version
Broken: version: 2.80 (sub 75), branch: master, commit date: 2019-07-29 14:47, hash: 0666ece2e2
Worked: 2.79

Short description of error
Particles created inside a moving collision object can "leak out" until the object comes to rest.
This may be related to #68128, but I wasn't able to get the .blend files there to work properly with the Blender versions near the guilty commit (see below).
The guilty commit is 0666ece2. The particles stay inside the sphere before this commit and leak out after.

Exact steps for others to reproduce the error
Open:
T71620_test.blend

Run animation.
Notice how many particles fail the collision.

In blender 2.79 none of the particles fail.

**System Information** Operating system: Linux-4.15.0-70-generic-x86_64-with-debian-buster-sid 64 Bits Graphics card: GeForce GTX 1080/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 440.26 **Blender Version** Broken: version: 2.80 (sub 75), branch: master, commit date: 2019-07-29 14:47, hash: 0666ece2e2 Worked: 2.79 **Short description of error** Particles created inside a moving collision object can "leak out" until the object comes to rest. This *may* be related to #68128, but I wasn't able to get the .blend files there to work properly with the Blender versions near the guilty commit (see below). The guilty commit is 0666ece2. The particles stay inside the sphere before this commit and leak out after. **Exact steps for others to reproduce the error** Open: [T71620_test.blend](https://archive.blender.org/developer/F8099781/T71620_test.blend) - Run animation. - Notice how many particles fail the collision. In blender 2.79 none of the particles fail.

Kevin Buhr commented

2019-11-16 04:24:49 +01:00

Added subscriber: @buhr

Kevin Buhr commented

2019-11-16 04:25:45 +01:00

T71620_test.blend

[T71620_test.blend](https://archive.blender.org/developer/F8099781/T71620_test.blend)

Kevin Buhr commented

2019-11-16 05:32:25 +01:00

The specific part of the commit that seems to cause the problem is this bit:

diff --git a/source/blender/blenkernel/intern/collision.c b/source/blender/blenkernel/intern/collision.c
index 183a4f9a181..d0eac3bb713 100644
--- a/source/blender/blenkernel/intern/collision.c
+++ b/source/blender/blenkernel/intern/collision.c
@@ -74,7 +89,7 @@ Collision modifier code start
 /* step is limited from 0 (frame start position) to 1 (frame end position) */
 void collision_move_object(CollisionModifierData *collmd, float step, float prevstep)
 {
-	float tv[3] = {0, 0, 0};
+	float oldx[3];
 	unsigned int i = 0;
 
 	/* the collider doesn't move this frame */
@@ -87,15 +102,14 @@ void collision_move_object(CollisionModifierData *collmd, float step, float prev
 	}
 
 	for (i = 0; i < collmd->mvert_num; i++) {
-		sub_v3_v3v3(tv, collmd->xnew[i].co, collmd->x[i].co);
-		VECADDS(collmd->current_x[i].co, collmd->x[i].co, tv, prevstep);
-		VECADDS(collmd->current_xnew[i].co, collmd->x[i].co, tv, step);
-		sub_v3_v3v3(collmd->current_v[i].co, collmd->current_xnew[i].co, collmd->current_x[i].co);
+		interp_v3_v3v3(oldx, collmd->x[i].co, collmd->xnew[i].co, prevstep);
+		interp_v3_v3v3(collmd->current_x[i].co, collmd->x[i].co, collmd->xnew[i].co, step);
+		sub_v3_v3v3(collmd->current_v[i].co, collmd->current_x[i].co, oldx);
 	}
 
 	bvhtree_update_from_mvert(
-	        collmd->bvhtree, collmd->current_x, collmd->current_xnew,
-	        collmd->tri, collmd->tri_num, true);
+	        collmd->bvhtree, collmd->current_x, NULL,
+	        collmd->tri, collmd->tri_num, false);
 }
 
 BVHTree *bvhtree_build_from_mvert(

If this change is backed out (or altered to preserve the original semantics), particle collisions stop leaking. The trouble is, I can't tell what the motivation was for making this change in the first place.

The specific part of the commit that seems to cause the problem is this bit: ``` diff --git a/source/blender/blenkernel/intern/collision.c b/source/blender/blenkernel/intern/collision.c index 183a4f9a181..d0eac3bb713 100644 --- a/source/blender/blenkernel/intern/collision.c +++ b/source/blender/blenkernel/intern/collision.c @@ -74,7 +89,7 @@ Collision modifier code start /* step is limited from 0 (frame start position) to 1 (frame end position) */ void collision_move_object(CollisionModifierData *collmd, float step, float prevstep) { - float tv[3] = {0, 0, 0}; + float oldx[3]; unsigned int i = 0; /* the collider doesn't move this frame */ @@ -87,15 +102,14 @@ void collision_move_object(CollisionModifierData *collmd, float step, float prev } for (i = 0; i < collmd->mvert_num; i++) { - sub_v3_v3v3(tv, collmd->xnew[i].co, collmd->x[i].co); - VECADDS(collmd->current_x[i].co, collmd->x[i].co, tv, prevstep); - VECADDS(collmd->current_xnew[i].co, collmd->x[i].co, tv, step); - sub_v3_v3v3(collmd->current_v[i].co, collmd->current_xnew[i].co, collmd->current_x[i].co); + interp_v3_v3v3(oldx, collmd->x[i].co, collmd->xnew[i].co, prevstep); + interp_v3_v3v3(collmd->current_x[i].co, collmd->x[i].co, collmd->xnew[i].co, step); + sub_v3_v3v3(collmd->current_v[i].co, collmd->current_x[i].co, oldx); } bvhtree_update_from_mvert( - collmd->bvhtree, collmd->current_x, collmd->current_xnew, - collmd->tri, collmd->tri_num, true); + collmd->bvhtree, collmd->current_x, NULL, + collmd->tri, collmd->tri_num, false); } BVHTree *bvhtree_build_from_mvert( ``` If this change is backed out (or altered to preserve the original semantics), particle collisions stop leaking. The trouble is, I can't tell what the motivation was for making this change in the first place.

Philipp Oeser commented

2019-11-16 08:20:29 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2019-11-18 12:09:15 +01:00

note there was also a problem regarding hair collisions that was caused by culprit commit 0666ece2 [this was hopefully fixed in e52ad1835a, might give a hint on this issue as well...]

note there was also a problem regarding hair collisions that was caused by culprit commit 0666ece2 [this was hopefully fixed in e52ad1835a, might give a hint on this issue as well...]

Philipp Oeser commented

2019-11-18 14:09:18 +01:00

Seems like this will do [but atm. I am on the same page as @buhr -- in that it is unclear why that change was made for cloth]
P1165: T71620_snippet



diff --git a/source/blender/blenkernel/intern/collision.c b/source/blender/blenkernel/intern/collision.c
index 91d66e16dde..51d0c4703d4 100644
--- a/source/blender/blenkernel/intern/collision.c
+++ b/source/blender/blenkernel/intern/collision.c
@@ -98,7 +98,7 @@ void collision_move_object(CollisionModifierData *collmd, float step, float prev
   }
 
   bvhtree_update_from_mvert(
-      collmd->bvhtree, collmd->current_x, NULL, collmd->tri, collmd->tri_num, false);
+      collmd->bvhtree, collmd->current_x, collmd->current_xnew, collmd->tri, collmd->tri_num, true);
 }
 
 BVHTree *bvhtree_build_from_mvert(const MVert *mvert,

(basically it looks like [for cloth?], the points are not considered moving anymore -- thus their KDTree node is not expanded anymore... needs some further investigation...)
(personally I would consider this a High priority, but the particle system is EOL... so will leave up to others...)

Seems like this will do [but atm. I am on the same page as @buhr -- in that it is unclear why that change was made for cloth] [P1165: T71620_snippet](https://archive.blender.org/developer/P1165.txt) ``` diff --git a/source/blender/blenkernel/intern/collision.c b/source/blender/blenkernel/intern/collision.c index 91d66e16dde..51d0c4703d4 100644 --- a/source/blender/blenkernel/intern/collision.c +++ b/source/blender/blenkernel/intern/collision.c @@ -98,7 +98,7 @@ void collision_move_object(CollisionModifierData *collmd, float step, float prev } bvhtree_update_from_mvert( - collmd->bvhtree, collmd->current_x, NULL, collmd->tri, collmd->tri_num, false); + collmd->bvhtree, collmd->current_x, collmd->current_xnew, collmd->tri, collmd->tri_num, true); } BVHTree *bvhtree_build_from_mvert(const MVert *mvert, ``` (basically it looks like [for cloth?], the points are not considered moving anymore -- thus their KDTree node is not expanded anymore... needs some further investigation...) (personally I would consider this a High priority, but the particle system is EOL... so will leave up to others...)

Philipp Oeser commented

2019-11-18 15:29:07 +01:00

Added subscribers: @LucaRood-3, @brecht

Philipp Oeser commented

2019-11-18 15:29:07 +01:00

Been reading code a bit, I think this might boil down to this statement from D3712: Cloth: improved collisions:

Use static BVH overlap, to reduce false positives, as moving collisions are not computed anyway.

This might be true for cloth, it seems it is not for particles (havent checked hair here, but this might be the reason for some reports on hair...)

CC @LucaRood-3
CC @brecht`

Been reading code a bit, I think this might boil down to this statement from [D3712: Cloth: improved collisions](https://archive.blender.org/developer/D3712): > Use static BVH overlap, to reduce false positives, as moving collisions are not computed anyway. This might be true for cloth, it seems it is **not** for particles (havent checked hair here, but this might be the reason for some reports on hair...) CC @LucaRood-3 CC @brecht`

Germano Cavalcante changed title from ~~D3712 (Cloth: collision improvements) breaks particle collisions with moving meshes~~ to rB0666ece2e2f9 (Cloth: collision improvements) breaks particle collisions with moving meshes

2020-01-10 15:33:00 +01:00

Philipp Oeser commented

2020-01-10 15:46:18 +01:00

Added subscriber: @mano-wii

Philipp Oeser commented

2020-01-10 15:46:18 +01:00

@LucaRood-3 : @mano-wii suggested to isolate the change of P1165 to only affect particles, doublecheck and maybe commit. Opinions?

@LucaRood-3 : @mano-wii suggested to isolate the change of [P1165](https://archive.blender.org/developer/P1165.txt) to only affect particles, doublecheck and maybe commit. Opinions?

Luca Rood commented

2020-01-11 16:08:33 +01:00

Sorry for the late reaction. I somehow completely missed this task before.

Indeed I seem to have overlooked particle collisions when implementing that optimisation.
I now submitted D6560 which is basically what @lichtwerk and @mano-wii proposed, but with the added update of current_xnew and current_x.

Btw, for future reference, this issue was caused specifically by fe9debc47b, which is one of the commits that eventually got squashed into 0666ece2.

Sorry for the late reaction. I somehow completely missed this task before. Indeed I seem to have overlooked particle collisions when implementing that optimisation. I now submitted [D6560](https://archive.blender.org/developer/D6560) which is basically what @lichtwerk and @mano-wii proposed, but with the added update of `current_xnew` and `current_x`. Btw, for future reference, this issue was caused specifically by fe9debc47b, which is one of the commits that eventually got squashed into 0666ece2.

blender-admin commented

2020-01-12 17:08:01 +01:00

This issue was referenced by 0ef881cc57

This issue was referenced by 0ef881cc57829471c441367ada6bf88119eaf26a

Luca Rood commented

2020-01-12 17:26:44 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Luca Rood closed this issue

2020-01-12 17:26:44 +01:00

Luca Rood self-assigned this 2020-01-12 17:26:44 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

rB0666ece2e2f9 (Cloth: collision improvements) breaks particle collisions with moving meshes #71620