Regression: QuadriFlow Remesh Crash #105953

New Issue

Philipp Oeser · 2023-03-21T13:50:12+01:00

Philipp Oeser commented

2023-03-21 13:50:12 +01:00

System Information
Operating system: Linux-6.1.18-200.fc37.x86_64-x86_64-with-glibc2.36 64 Bits
Graphics card: AMD Radeon Graphics (renoir, LLVM 15.0.7, DRM 3.49, 6.1.18-200.fc37.x86_64) AMD 4.6 (Core Profile) Mesa 22.3.7

Blender Version
Broken: version: 3.6.0 Alpha, branch: main, commit date: 2023-03-20 00:12, hash: 7cda559d7c1b
Broken: 3.4
Worked: 3.3 LTS

Caused by 511ac66dab

Short description of error
QuadriFlow Remesh crashes with a somewhat large mesh
Spotted while investigating #105164 (which is related, but not a regression like the case presented here)

Exact steps for others to reproduce the error
I cleaned up the file from #105164 a bit (removed materials, attributes, UVMaps, nonmanifold parts etc.) to rule out other possible errors.

This now has 4 separate parts.

joining does not result in nonmanifold geo
if you join any three of them, a QuadriFlow Remesh afterwards to 40k faces works on my side
if you join all 4 of them, a QuadriFlow Remesh afterwards to 40k faces crashes

repro_cleanup.blend

OR, if you just want the file that was working before 511ac66dab (and is now crashing), use the already joined file

So
repro_cleanup_joined.blend

open the file
QuadriFlow Remesh operator set to 40k faces
crash

**System Information** Operating system: Linux-6.1.18-200.fc37.x86_64-x86_64-with-glibc2.36 64 Bits Graphics card: AMD Radeon Graphics (renoir, LLVM 15.0.7, DRM 3.49, 6.1.18-200.fc37.x86_64) AMD 4.6 (Core Profile) Mesa 22.3.7 **Blender Version** Broken: version: 3.6.0 Alpha, branch: main, commit date: 2023-03-20 00:12, hash: `7cda559d7c1b` Broken: 3.4 Worked: 3.3 LTS Caused by 511ac66dab **Short description of error** QuadriFlow Remesh crashes with a somewhat large mesh Spotted while investigating #105164 (which is related, but not a regression like the case presented here) **Exact steps for others to reproduce the error** I cleaned up the file from #105164 a bit (removed materials, attributes, UVMaps, nonmanifold parts etc.) to rule out other possible errors. This now has 4 separate parts. - joining does not result in nonmanifold geo - if you join any three of them, a QuadriFlow Remesh afterwards to 40k faces works on my side - if you join all 4 of them, a QuadriFlow Remesh afterwards to 40k faces crashes [repro_cleanup.blend](/attachments/f9eec6ed-a46e-4e66-a61a-99efcda0ed82) OR, if you just want the file that was working before 511ac66dab (and is now crashing), use the already joined file So [repro_cleanup_joined.blend](/attachments/ad09bd6e-3cab-4f34-8b01-13d6ff632db9) - open the file - QuadriFlow Remesh operator set to 40k faces - crash

repro_cleanup_joined.blend

75 MiB

Philipp Oeser added the

labels 2023-03-21 13:50:48 +01:00

Philipp Oeser commented

2023-03-21 13:51:20 +01:00

CC @HooglyBoogly
CC @ZedDB

CC @HooglyBoogly CC @ZedDB

Philipp Oeser referenced this issue

2023-03-21 13:52:09 +01:00

QuadriFlow Remesh Crash #105164

Hans Goudey added

and removed

labels 2023-03-21 14:46:23 +01:00

Hans Goudey commented

2023-03-22 12:52:53 +01:00

In a quest for more information I ran this in a debug build. 1 hour later it isn't finished with 40k verts, so I'll try something else. It does look like there's a bunch more mesh copying going on than necessary in this operator, but I didn't find that was the problem.

Jacques Lucke commented

2023-03-22 17:52:44 +01:00

Can someone confirm that this fixes it?

diff --git a/extern/quadriflow/src/hierarchy.cpp b/extern/quadriflow/src/hierarchy.cpp
index 8cc41da23d0..0bf2ba53b23 100644
--- a/extern/quadriflow/src/hierarchy.cpp
+++ b/extern/quadriflow/src/hierarchy.cpp
@@ -269,7 +269,7 @@ void Hierarchy::DownsampleGraph(const AdjacentMatrix adj, const MatrixXd& V, con
         for (auto it = ad.begin(); it != ad.end(); ++it, ++entry_it) {
             int k = it->id;
             double dp = N.col(i).dot(N.col(k));
-            double ratio = A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i]);
+            double ratio = (A[i] == 0.0f || A[k] == 0.0f) ? (A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i])) : 1.0f;
             *entry_it = Entry(i, k, dp * ratio);
         }
     }

I got /home/jacques/blender/blender/extern/quadriflow/src/hierarchy.cpp:272:48: runtime error: division by zero from ASAN.

Can someone confirm that this fixes it? ``` diff --git a/extern/quadriflow/src/hierarchy.cpp b/extern/quadriflow/src/hierarchy.cpp index 8cc41da23d0..0bf2ba53b23 100644 --- a/extern/quadriflow/src/hierarchy.cpp +++ b/extern/quadriflow/src/hierarchy.cpp @@ -269,7 +269,7 @@ void Hierarchy::DownsampleGraph(const AdjacentMatrix adj, const MatrixXd& V, con for (auto it = ad.begin(); it != ad.end(); ++it, ++entry_it) { int k = it->id; double dp = N.col(i).dot(N.col(k)); - double ratio = A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i]); + double ratio = (A[i] == 0.0f || A[k] == 0.0f) ? (A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i])) : 1.0f; *entry_it = Entry(i, k, dp * ratio); } } ``` I got `/home/jacques/blender/blender/extern/quadriflow/src/hierarchy.cpp:272:48: runtime error: division by zero` from ASAN.

Hans Goudey commented

2023-03-22 18:34:05 +01:00

This fixed the issue for me too. Thanks!

Philipp Oeser commented

2023-03-23 09:42:42 +01:00

Can also confirm this fixes it for me (#105164 still crashes though, but that is another story...)

Erik Abrahamsson commented

2023-03-24 15:16:10 +01:00

Sorry but can someone explain how that patch works?
To me it looks like it checks if A[i] or A[k] is 0.0f and if it is it still runs the division (which will always be divide by zero or 0.0f) and if not, the ratio is 1.0f.

Sorry but can someone explain how that patch works? To me it looks like it checks if `A[i]` or `A[k]` is 0.0f and if it is it still runs the division (which will always be divide by zero or 0.0f) and if not, the ratio is 1.0f.

Jacques Lucke commented

2023-03-27 11:04:38 +02:00

@erik85 It probably makes more sense when you have a closer look at the parenthesis (nested ternary conditions are hard to read)

Erik Abrahamsson commented

2023-03-27 11:21:15 +02:00

@erik85 It probably makes more sense when you have a closer look at the parenthesis (nested ternary conditions are hard to read)

Hmm, no I don't see it. I must be misunderstanding the whole point of that.

(A[i] == 0.0f || A[k] == 0.0f)
?
(A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i]))
:
1.0f

> @erik85 It probably makes more sense when you have a closer look at the parenthesis (nested ternary conditions are hard to read) Hmm, no I don't see it. I must be misunderstanding the whole point of that. ``` (A[i] == 0.0f || A[k] == 0.0f) ? (A[i] > A[k] ? (A[i] / A[k]) : (A[k] / A[i])) : 1.0f ```

Jacques Lucke commented

2023-03-27 11:26:46 +02:00

Hm, you're right, that is even more confusing. Guess it cancels the problem out somehow, don't ask me why. It works the the "correct" condition as well though.

Jacques Lucke referenced a pull request that will close this issue

2023-03-27 11:27:30 +02:00

Fix #105953: division by zero in quadriflow leads to crash #106165

Jacques Lucke self-assigned this 2023-03-27 14:49:22 +02:00

Jacques Lucke referenced this issue from a commit

2023-03-28 14:25:32 +02:00

Fix #105953: division by zero in quadriflow leads to crash

Jacques Lucke closed this issue

2023-03-28 14:25:34 +02:00

Blender Bot added

Status

Resolved