From 5991bfdb3a58fc313f6ffb469eae2b1ecfa14a19 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sat, 14 Dec 2024 20:51:12 +0530 Subject: [PATCH 1/7] Make type annotation on `points` more abstract --- CairoMakie/src/utils.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl index bc99af64138..fc252f4587c 100644 --- a/CairoMakie/src/utils.jl +++ b/CairoMakie/src/utils.jl @@ -193,7 +193,7 @@ 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} # If colors are defined per point they need to be interpolated like positions # at clip planes per_point_colors = colors isa AbstractArray @@ -202,7 +202,7 @@ function project_line_points(scene, plot::T, positions, colors, linewidths) wher 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) + points = Makie.apply_transform(transform_func(plot), positions, space)::AbstractVector{Point{N, FT}} f32convert = Makie.f32_convert_matrix(scene.float32convert, space) transform = Makie.space_to_clip(scene.camera, space) * f32convert * model clip_points = map(points) do point From 6bc043c8a1564eb68cf8189766b54a8e8b99b18f Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sun, 15 Dec 2024 09:26:44 +0530 Subject: [PATCH 2/7] Implement the function barrier approach --- CairoMakie/src/utils.jl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl index fc252f4587c..c81e0c9fe96 100644 --- a/CairoMakie/src/utils.jl +++ b/CairoMakie/src/utils.jl @@ -194,6 +194,15 @@ end 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. + points = Makie.apply_transform(transform_func(plot), positions, space)::AbstractVector{Point{N, FT}} + 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 @@ -201,8 +210,6 @@ function project_line_points(scene, plot::T, positions::AbstractArray{<: Makie.V 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)::AbstractVector{Point{N, FT}} f32convert = Makie.f32_convert_matrix(scene.float32convert, space) transform = Makie.space_to_clip(scene.camera, space) * f32convert * model clip_points = map(points) do point From 8b26286dabc07af1385c95eed3cb1a73f902b393 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sun, 15 Dec 2024 09:31:49 +0530 Subject: [PATCH 3/7] Fix typo --- CairoMakie/src/utils.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl index c81e0c9fe96..775c3cfe06d 100644 --- a/CairoMakie/src/utils.jl +++ b/CairoMakie/src/utils.jl @@ -197,7 +197,9 @@ function project_line_points(scene, plot::T, positions::AbstractArray{<: Makie.V # 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)::AbstractVector{Point{N, FT}} + return project_transformed_line_points(scene, plot, points, colors, linewidths) end From c0e9a23ae4fc8a01fb3733b449e5fb678afb27df Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sun, 15 Dec 2024 09:31:58 +0530 Subject: [PATCH 4/7] Add a test to the CairoMakie tests --- CairoMakie/test/runtests.jl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl index a506645e4e3..0813c6d337e 100644 --- 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 \ No newline at end of file From 95c10865faa8199bf2a37e1be937aa1733e04735 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sun, 15 Dec 2024 09:34:46 +0530 Subject: [PATCH 5/7] Add a changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdfe0a97768..1defa708176 100644 --- 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 points were not handled correctly in CairoMakie [#4668](https://github.com/MakieOrg/Makie.jl/pull/4668). ## [0.21.18] - 2024-12-12 From 51fc4a582dcd32511e366f5b723d0dafc654d3ce Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Sun, 15 Dec 2024 15:16:23 +0530 Subject: [PATCH 6/7] Remove type annotation to enable 2D->3D --- CairoMakie/src/utils.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl index 775c3cfe06d..766d9c2a2c1 100644 --- a/CairoMakie/src/utils.jl +++ b/CairoMakie/src/utils.jl @@ -198,7 +198,7 @@ function project_line_points(scene, plot::T, positions::AbstractArray{<: Makie.V # 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)::AbstractVector{Point{N, FT}} + points = Makie.apply_transform(transform_func(plot), positions, space) return project_transformed_line_points(scene, plot, points, colors, linewidths) end From 367fcce5bae016d630b3a244441d11d244e18a75 Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Sun, 15 Dec 2024 14:03:14 +0100 Subject: [PATCH 7/7] mention lines in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1defa708176..da6ca7d4c11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +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 points were not handled correctly in CairoMakie [#4668](https://github.com/MakieOrg/Makie.jl/pull/4668). +- 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