fix contour boundingbox with NaNs (#3210)

CairoMakie/src/primitives.jl

@@ -211,7 +211,7 @@ function draw_multi(primitive::Lines, ctx, positions, colors::AbstractArray, lin
         # first is nan, do nothing
     end
 
-    for i in eachindex(positions)[2:end]
+    for i in eachindex(positions)[begin+1:end]
         this_position = positions[i]
         this_color = colors[i]
         this_nan = isnan(this_position)
@@ -524,7 +524,6 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Text
     nothing
 end
 
-
 function draw_glyph_collection(
         scene, ctx, positions, glyph_collections::AbstractArray, rotation,
         model::Mat, space, markerspace, offset, transformation, transform_marker
@@ -587,6 +586,11 @@ function draw_glyph_collection(
         # offsets and scale apply in markerspace
         gp3 = glyph_pos[Vec(1, 2, 3)] ./ glyph_pos[4] .+ model33 * (glyphoffset .+ p3_offset)
 
+        if any(isnan, gp3)
+            Cairo.restore(ctx)
+            return
+        end
+
         scale3 = scale isa Number ? Point3f(scale, scale, 0) : to_ndim(Point3f, scale, 0)
 
         # the CairoMatrix is found by transforming the right and up vector

src/basic_recipes/contours.jl

@@ -62,6 +62,7 @@ angle(p1::Union{Vec2f,Point2f}, p2::Union{Vec2f,Point2f})::Float32 =
 
 function label_info(lev, vertices, col)
     mid = ceil(Int, 0.5f0 * length(vertices))
+    # take 3 pts around half segment
     pts = (vertices[max(firstindex(vertices), mid - 1)], vertices[mid], vertices[min(mid + 1, lastindex(vertices))])
     (
         lev,
@@ -196,11 +197,9 @@ end
 function has_changed(old_args, new_args)
     length(old_args) === length(new_args) || return true
     for (old, new) in zip(old_args, new_args)
-        if old != new
-            return true
-        end
+        old != new && return true
     end
-    return false
+    false
 end
 
 function plot!(plot::T) where T <: Union{Contour, Contour3d}
@@ -230,8 +229,7 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
         # We need to copy, since the values may get mutated in place
         if has_changed(old_values, args)
             old_values = map(copy, args)
-            _points, _colors, _lev_pos_col = contourlines(args..., T)
-            points[] = _points; colors[] = _colors; lev_pos_col[] = _lev_pos_col
+            points[], colors[], lev_pos_col[] = contourlines(args..., T)
             return
         end
     end
@@ -274,7 +272,10 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
             push!(col, labelcolor === nothing ? color : to_color(labelcolor))
             push!(rot, rot_from_vert)
             push!(lbl, labelformatter(lev))
-            push!(pos, p1)
+            p = p2  # try to position label around center
+            isnan(p) && (p = p1)
+            isnan(p) && (p = p3)
+            push!(pos, p)
         end
         notify(texts.text)
         return
@@ -285,12 +286,16 @@ function plot!(plot::T) where T <: Union{Contour, Contour3d}
         return broadcast(texts.plots[1][1].val, texts.positions.val, texts.rotation.val) do gc, pt, rot
             # drop the depth component of the bounding box for 3D
             px_pos = project(scene, apply_transform(transform_func(plot), pt, space))
-            Rect2f(boundingbox(gc, to_ndim(Point3f, px_pos, 0f0), to_rotation(rot)))
+            bb = unchecked_boundingbox(gc, to_ndim(Point3f, px_pos, 0f0), to_rotation(rot))
+            isfinite_rect(bb) || return Rect2f()
+            Rect2f(bb)
         end
     end
 
     masked_lines = lift(labels, bboxes, points) do labels, bboxes, segments
         labels || return segments
+        # simple heuristic to turn off masking segments (≈ less than 10 pts per contour)
+        count(isnan, segments) > length(segments) / 10 && return segments
         n = 1
         bb = bboxes[n]
         nlab = length(bboxes)

src/layouting/boundingbox.jl

@@ -48,14 +48,11 @@ end
 
 _inkboundingbox(ext::GlyphExtent) = ext.ink_bounding_box
 
-function boundingbox(glyphcollection::GlyphCollection, position::Point3f, rotation::Quaternion)
-    return boundingbox(glyphcollection, rotation) + position
-end
+unchecked_boundingbox(glyphcollection::GlyphCollection, position::Point3f, rotation::Quaternion) =
+    unchecked_boundingbox(glyphcollection, rotation) + position
 
-function boundingbox(glyphcollection::GlyphCollection, rotation::Quaternion)
-    if isempty(glyphcollection.glyphs)
-        return Rect3f(Point3f(0), Vec3f(0))
-    end
+function unchecked_boundingbox(glyphcollection::GlyphCollection, rotation::Quaternion)
+    isempty(glyphcollection.glyphs) && return Rect3f(Point3f(0), Vec3f(0))
 
     glyphorigins = glyphcollection.origins
     glyphbbs = gl_bboxes(glyphcollection)
@@ -69,25 +66,27 @@ function boundingbox(glyphcollection::GlyphCollection, rotation::Quaternion)
             bb = union(bb, charbb)
         end
     end
-    !isfinite_rect(bb) && error("Invalid text boundingbox")
     return bb
 end
 
-function boundingbox(layouts::AbstractArray{<:GlyphCollection}, positions, rotations)
-    if isempty(layouts)
-        return Rect3f((0, 0, 0), (0, 0, 0))
-    else
-        bb = Rect3f()
-        broadcast_foreach(layouts, positions, rotations) do layout, pos, rot
-            if !isfinite_rect(bb)
-                bb = boundingbox(layout, pos, rot)
-            else
-                bb = union(bb, boundingbox(layout, pos, rot))
-            end
+function unchecked_boundingbox(layouts::AbstractArray{<:GlyphCollection}, positions, rotations)
+    isempty(layouts) && return Rect3f((0, 0, 0), (0, 0, 0))
+
+    bb = Rect3f()
+    broadcast_foreach(layouts, positions, rotations) do layout, pos, rot
+        if !isfinite_rect(bb)
+            bb = boundingbox(layout, pos, rot)
+        else
+            bb = union(bb, boundingbox(layout, pos, rot))
         end
-        !isfinite_rect(bb) && error("Invalid text boundingbox")
-        return bb
     end
+    return bb
+end
+
+function boundingbox(x::Union{GlyphCollection,AbstractArray{<:GlyphCollection}}, args...)
+    bb = unchecked_boundingbox(x, args...)
+    isfinite_rect(bb) || error("Invalid text boundingbox")
+    bb
 end
 
 function boundingbox(x::Text{<:Tuple{<:GlyphCollection}})

test/boundingboxes.jl

@@ -102,4 +102,13 @@ end
     bb = boundingbox(p)
     @test bb.origin ≈ Point3f(343.0, 345.0, 0)
     @test bb.widths ≈ Vec3f(32.24, 23.3, 0)
-end
+end
+
+@testset "invalid contour bounding box" begin
+    a = b = 1:3
+    levels = collect(1:3)
+    c = [0 1 2; 1 2 3; 4 5 NaN]
+    contour(a, b, c; levels, labels = true)
+    c = [0 1 2; 1 2 3; 4 5 Inf]
+    contour(a, b, c; levels, labels = true)
+end