Fix WGLMakie nan error (#4772)

* fix Planes normalizing 0 vecs to NaN vecs * add test * update changelog * add comment
MakieOrg · Feb 14, 2025 · c7eb320 · c7eb320
1 parent de99964
commit c7eb320
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - Fixed issue with `WGLMakie.voxels` not rendering on linux with firefox [#4756](https://github.com/MakieOrg/Makie.jl/pull/4756)
 - Cleaned up surface handling in GLMakie: Surface cells are now discarded when there is a nan in x, y or z. Fixed incorrect normal if x or y is nan [#4735](https://github.com/MakieOrg/Makie.jl/pull/4735)
 - Cleaned up `volume` plots: Added `:indexedabsorption` and `:additive` to WGLMakie, generalized `:mip` to include negative values, fixed missing conversions for rgba algorithms (`:additive`, `:absorptionrgba`), fixed missing conversion for `absorption` attribute & extended it to `:indexedabsorption` and `absorptionrgba`, added tests and improved docs. [#4726](https://github.com/MakieOrg/Makie.jl/pull/4726)
+- Fixed `Plane(Vec{N, T}(0), dist)` producing a `NaN` normal, which caused WGLMakie to break. (E.g. when rotating Axis3) [#4772](https://github.com/MakieOrg/Makie.jl/pull/4772)
 
 ## [0.22.1] - 2025-01-17
 

diff --git a/src/utilities/Plane.jl b/src/utilities/Plane.jl
@@ -5,7 +5,13 @@ struct Plane{N, T}
     distance::T
 
     function Plane{N, T}(normal::Vec{N, T}, distance::T) where {N, T <: Real}
-        return new{N, T}(normalize(normal), distance)
+        # Functions using Plane assume `normal` to be normalized. 
+        # `normalize()` turns 0 vectors into NaN vectors which we don't want, 
+        # so we explicitly handle normalization here
+        n = norm(normal)
+        ϵ = 100 * max(eps.(normal)...)
+        normalized = ifelse(n > ϵ, normal / n, Vec{N, T}(0))
+        return new{N, T}(normalized, distance)
     end
 end
 
@@ -329,4 +335,4 @@ function to_clip_space(pv::Mat4, ipv::Mat4, plane::Plane3)
 
         return Plane(origin, normal)
     end
-end
+end
diff --git a/test/isolated/Plane.jl b/test/isolated/Plane.jl
@@ -6,7 +6,7 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
             plane = Plane(Point3{T}(1), Vec3{T}(1, 1, 0))
             @test plane isa Plane{3, T}
             @test plane.normal ≈ normalize(Vec3{T}(1, 1, 0))
-            @test plane.distance ≈ T(sqrt(2.0)) 
+            @test plane.distance ≈ T(sqrt(2.0))
 
             plane = Plane(Point2{T}(1), Vec2{T}(0, 1))
             @test plane isa Plane{2, T}
@@ -16,7 +16,7 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
             plane = Plane(Vec3{T}(1), 2.0)
             @test plane isa Plane{3, T}
             @test plane.normal ≈ normalize(Vec3{T}(1))
-            @test plane.distance ≈ T(2) 
+            @test plane.distance ≈ T(2)
 
             clip_box = Rect3f(Point3f(0), Vec3f(1))
             planes = Makie.planes(clip_box)
@@ -28,6 +28,11 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
                 Plane3f(Vec3f( 0, -1,  0), -1f0),
                 Plane3f(Vec3f( 0,  0, -1), -1f0)
             ]
+
+            plane = Plane(Vec3{T}(0), 2.0)
+            @test plane isa Plane{3, T}
+            @test plane.normal ≈ Vec3{T}(0)
+            @test plane.distance ≈ T(2)
         end
     end
 
@@ -107,7 +112,7 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
         plane = Plane3f(Point3f(0.5), Vec3f(0,0,1))
 
         # transform points
-        ds = let 
+        ds = let
             transformed = map(p -> Point3f(model * to_ndim(Point4f, p, 1)), ps)
             Makie.distance.((plane,), transformed)
         end
@@ -120,10 +125,10 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
         bbox = Rect3d(Point3d(-1), Vec3d(2))
         planes = [Plane3f(Point3f(0.5), Vec3f(-0.5))]
         @test Makie.apply_clipping_planes(planes, bbox) == bbox
-       
+
         planes = [Plane3f(Point3f(0, 0, 0.5), Vec3f(-0.0, -0.0, -0.5))]
         @test Makie.apply_clipping_planes(planes, bbox) == Rect3d(Point3d(-1), Vec3d(2,2,1.5))
-       
+
         planes = Makie.planes(Rect3f(Point3f(-0.5), Vec3f(1)))
         bb = Makie.apply_clipping_planes(planes, bbox)
         @test minimum(bb) ≈ Vec3f(-0.5)
@@ -138,7 +143,7 @@ using Makie: Plane, Plane3f, Point3d, Rect3d, Vec3d
         ps = [2f0 .* rand(Point3f) .- 1f0 for _ in 1:1000]
         planes = Makie.planes(Rect3f(Point3f(-0.5), Vec3f(1)))
         inside = sum(Makie.is_visible.((planes,), ps))
-        
+
         # COV_EXCL_START
         f, a, p = scatter(ps)
         Makie.update_state_before_display!(f)