Fix CairoMakie not being able to plot exotic point arrays in lines (#…

…4668) * Make type annotation on `points` more abstract * Implement the function barrier approach * Fix typo * Add a test to the CairoMakie tests * Add a changelog entry * Remove type annotation to enable 2D->3D * mention lines in changelog --------- Co-authored-by: Frederic Freyer <[email protected]>
MakieOrg · Dec 16, 2024 · 5c7aafa · 5c7aafa
1 parent 158fe66
commit 5c7aafa
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@
 - Added `transform_marker` attribute to meshscatter and changed the default behavior to not transform marker/mesh vertices [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
 - Fixed some issues with meshscatter not correctly transforming with transform functions and float32 rescaling [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
 - Fixed `poly` pipeline for 3D and/or Float64 polygons that begin from an empty vector [#4615](https://github.com/MakieOrg/Makie.jl/pull/4615).
+- Fixed an issue where `reinterpret`ed arrays of line points were not handled correctly in CairoMakie [#4668](https://github.com/MakieOrg/Makie.jl/pull/4668).
 
 ## [0.21.18] - 2024-12-12
 

diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl
@@ -193,16 +193,25 @@ end
 
 
 
-function project_line_points(scene, plot::T, positions, colors, linewidths) where {T <: Union{Lines, LineSegments}}
+function project_line_points(scene, plot::T, positions::AbstractArray{<: Makie.VecTypes{N, FT}}, colors, linewidths) where {T <: Union{Lines, LineSegments}, N, FT <: Real}
+
+    # Standard transform from input space to clip space
+    # Note that this is type unstable, so there is a function barrier in place.
+    space = (plot.space[])::Symbol
+    points = Makie.apply_transform(transform_func(plot), positions, space)
+
+    return project_transformed_line_points(scene, plot, points, colors, linewidths)
+end
+
+function project_transformed_line_points(scene, plot::T, points::AbstractArray{<: Makie.VecTypes{N, FT}}, colors, linewidths) where {T <: Union{Lines, LineSegments}, N, FT <: Real}
+    # Note that here, `points` has already had `transform_func` applied.
     # If colors are defined per point they need to be interpolated like positions
     # at clip planes
     per_point_colors = colors isa AbstractArray
     per_point_linewidths = (T <: Lines) && (linewidths isa AbstractArray)
 
     space = (plot.space[])::Symbol
     model = (plot.model[])::Mat4d
-    # Standard transform from input space to clip space
-    points = Makie.apply_transform(transform_func(plot), positions, space)::typeof(positions)
     f32convert = Makie.f32_convert_matrix(scene.float32convert, space)
     transform = Makie.space_to_clip(scene.camera, space) * f32convert * model
     clip_points = map(points) do point

diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl
@@ -304,4 +304,15 @@ end
     ps, _, _ = CairoMakie.project_line_points(a.scene, ls, ls_points, nothing, nothing)
     @test length(ps) >= 6 # at least 6 points: [2,3,3,4,4,5]
     @test all(ref -> findfirst(p -> isapprox(p, ref, atol = 1e-4), ps) !== nothing, necessary_points)
+
+    # Check that `reinterpret`ed arrays of points are handled correctly
+    # ref. https://github.com/MakieOrg/Makie.jl/issues/4661
+
+    data = reinterpret(Point2f, rand(Point2f, 10) .=> rand(Point2f, 10))
+
+    f, a, p = lines(data)
+    Makie.update_state_before_display!(f)
+    ps, _, _ = @test_nowarn CairoMakie.project_line_points(a.scene, p, data, nothing, nothing)
+    @test length(ps) == length(data) # this should never clip!
+
 end