Merge branch 'master' into jw/linux_reuse_empty

CHANGELOG.md

@@ -20,7 +20,10 @@
 - 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).
-- empty! GLMakie screen instead of closing, fixing issue with resetted window position [#3881](https://github.com/MakieOrg/Makie.jl/pull/3881)
+- `empty!` GLMakie screen instead of closing, fixing issue with resetted window position [#3881](https://github.com/MakieOrg/Makie.jl/pull/3881)
+- Fixed gaps in corners of `poly(Rect2(...))` stroke [#4664](https://github.com/MakieOrg/Makie.jl/pull/4664)
+- 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).
+- Fixed various issues with `markerspace = :data`, `transform_marker = true` and `rotation` for scatter in CairoMakie (incorrect marker transformations, ignored transformations, Cairo state corruption) [#4663](https://github.com/MakieOrg/Makie.jl/pull/4663)
 
 ## [0.21.18] - 2024-12-12
 

CairoMakie/src/cairo-extension.jl

@@ -28,6 +28,14 @@ function cairo_font_face_destroy(font_face)
     )
 end
 
+function cairo_transform(ctx, cairo_matrix)
+    ccall(
+        (:cairo_transform, Cairo.libcairo),
+        Cvoid, (Ptr{Cvoid}, Ptr{Cvoid}),
+        ctx.ptr, Ref(cairo_matrix)
+    )
+end
+
 function set_ft_font(ctx, font)
 
     font_face = Base.@lock font.lock ccall(

CairoMakie/src/precompiles.jl

@@ -18,7 +18,7 @@ end
 precompile(openurl, (String,))
 precompile(draw_atomic_scatter, (Scene, Cairo.CairoContext, Tuple{typeof(identity),typeof(identity)},
                     Vector{ColorTypes.RGBA{Float32}}, Vec{2,Float32}, ColorTypes.RGBA{Float32},
-                    Float32, BezierPath, Vec{2,Float32}, Quaternionf,
+                    Float32, BezierPath, Vec{3,Float32}, Quaternionf,
                     Mat4f, Vector{Point{2,Float32}},
                     Mat4f, Makie.FreeTypeAbstraction.FTFont, Symbol,
-                    Symbol))
+                    Symbol, Vector{Plane3f}, Bool))

CairoMakie/src/primitives.jl

@@ -325,77 +325,87 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scat
     markerspace = primitive.markerspace[]
     space = primitive.space[]
     transfunc = Makie.transform_func(primitive)
+    billboard = primitive.rotation[] isa Billboard
 
     return draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker,
                                marker_offset, rotation, model, positions, size_model, font, markerspace,
-                               space, clip_planes)
+                               space, clip_planes, billboard)
 end
 
 function draw_atomic_scatter(
         scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth,
         marker, marker_offset, rotation, model, positions, size_model, font,
-        markerspace, space, clip_planes
+        markerspace, space, clip_planes, billboard
     )
 
     transformed = apply_transform(transfunc, positions, space)
     indices = unclipped_indices(to_model_space(model, clip_planes), transformed, space)
-    projected_positions = project_position(scene, space, transformed, indices, model)
+    transform = Makie.clip_to_space(scene.camera, markerspace) *
+        Makie.space_to_clip(scene.camera, space) *
+        Makie.f32_convert_matrix(scene.float32convert, space) *
+        model
+    model33 = size_model[Vec(1,2,3), Vec(1,2,3)]
 
-    Makie.broadcast_foreach_index(projected_positions, indices, colors, markersize, strokecolor,
+    Makie.broadcast_foreach_index(view(transformed, indices), indices, colors, markersize, strokecolor,
             strokewidth, marker, marker_offset, remove_billboard(rotation)) do pos, col,
             markersize, strokecolor, strokewidth, m, mo, rotation
 
         isnan(pos) && return
         isnan(rotation) && return # matches GLMakie
+        isnan(markersize) && return
 
-        scale = project_scale(scene, markerspace, markersize, size_model)
-        offset = project_scale(scene, markerspace, mo, size_model)
+        p4d = transform * to_ndim(Point4d, to_ndim(Point3d, pos, 0), 1)
+        o = p4d[Vec(1, 2, 3)] ./ p4d[4] .+ model33 * to_ndim(Vec3d, mo, 0)
+        proj_pos, mat, jl_mat = project_marker(scene, markerspace, o,
+            markersize, rotation, size_model, billboard)
 
-        Cairo.set_source_rgba(ctx, rgbatuple(col)...)
+        # mat and jl_mat are the same matrix, once as a CairoMatrix, once as a Mat2f
+        # They both describe an approximate basis transformation matrix from
+        # marker space to pixel space with scaling appropriate to markersize.
+        # Markers that can be drawn from points/vertices of shape (e.g. Rect)
+        # could be projected more accurately by projecting each point individually
+        # and then building the shape.
 
+        # Enclosed area of the marker must be at least 1 pixel?
+        (abs(det(jl_mat)) < 1) && return
+
+        Cairo.set_source_rgba(ctx, rgbatuple(col)...)
         Cairo.save(ctx)
-        # Setting a markersize of 0.0 somehow seems to break Cairos global state?
-        # At least it stops drawing any marker afterwards
-        # TODO, maybe there's something wrong somewhere else?
-        if !(isnan(scale) || norm(scale) ≈ 0.0)
-            if m isa Char
-                draw_marker(ctx, m, best_font(m, font), pos, scale, strokecolor, strokewidth, offset, rotation)
-            else
-                draw_marker(ctx, m, pos, scale, strokecolor, strokewidth, offset, rotation)
-            end
+        if m isa Char
+            draw_marker(ctx, m, best_font(m, font), proj_pos, strokecolor, strokewidth, jl_mat, mat)
+        else
+            draw_marker(ctx, m, proj_pos, strokecolor, strokewidth, mat)
         end
         Cairo.restore(ctx)
     end
 
     return
 end
 
-function draw_marker(ctx, marker::Char, font, pos, scale, strokecolor, strokewidth, marker_offset, rotation)
+function draw_marker(ctx, marker::Char, font, pos, strokecolor, strokewidth, jl_mat, mat)
     cairoface = set_ft_font(ctx, font)
 
+    # The given pos includes the user position which corresponds to the center
+    # of the marker and the user marker_offset which may shift the position.
+    # At this point we still need to center the character we draw. For that we
+    # get the character boundingbox where (0,0) is the anchor point:
     charextent = Makie.FreeTypeAbstraction.get_extent(font, marker)
     inkbb = Makie.FreeTypeAbstraction.inkboundingbox(charextent)
 
-    # scale normalized bbox by font size
-    inkbb_scaled = Rect2f(origin(inkbb) .* scale, widths(inkbb) .* scale)
+    # And calculate an offset to the the center of the marker
+    centering_offset = Makie.origin(inkbb) .+ 0.5f0 .* widths(inkbb)
+    # which we then transform from marker space to screen space using the
+    # local coordinate transform derived by project_marker()
+    # (Need yflip because Cairo's y coordinates are reversed)
+    char_offset = Vec2f(jl_mat * ((1, -1) .* centering_offset))
 
-    # flip y for the centering shift of the character because in Cairo y goes down
-    centering_offset = Vec2f(1, -1) .* (-origin(inkbb_scaled) .- 0.5f0 .* widths(inkbb_scaled))
-    # this is the origin where we actually have to place the glyph so it can be centered
-    charorigin = pos .+ Vec2f(marker_offset[1], -marker_offset[2])
-    old_matrix = get_font_matrix(ctx)
-    set_font_matrix(ctx, scale_matrix(scale...))
-
-    # First, we translate to the point where the
-    # marker is supposed to go.
+    # The offset is then applied to pos and the marker placement is set
+    charorigin = pos - char_offset
     Cairo.translate(ctx, charorigin[1], charorigin[2])
-    # Then, we rotate the context by the
-    # appropriate amount,
-    Cairo.rotate(ctx, to_2d_rotation(rotation))
-    # and apply a centering offset to account for
-    # the fact that text is shown from the (relative)
-    # bottom left corner.
-    Cairo.translate(ctx, centering_offset[1], centering_offset[2])
+
+    # The font matrix takes care of rotation, scaling and shearing of the marker
+    old_matrix = get_font_matrix(ctx)
+    set_font_matrix(ctx, mat)
 
     Cairo.move_to(ctx, 0, 0)
     Cairo.text_path(ctx, string(marker))
@@ -409,56 +419,49 @@ function draw_marker(ctx, marker::Char, font, pos, scale, strokecolor, strokewid
     cairo_font_face_destroy(cairoface)
 
     set_font_matrix(ctx, old_matrix)
+    return
 end
 
-function draw_marker(ctx, ::Type{<: Circle}, pos, scale, strokecolor, strokewidth, marker_offset, rotation)
-    pos += Point2f(marker_offset[1], -marker_offset[2])
-
-    if scale[1] != scale[2]
-        old_matrix = Cairo.get_matrix(ctx)
-        Cairo.scale(ctx, scale[1], scale[2])
-        Cairo.translate(ctx, pos[1]/scale[1], pos[2]/scale[2])
-        Cairo.arc(ctx, 0, 0, 0.5, 0, 2*pi)
-    else
-        Cairo.arc(ctx, pos[1], pos[2], scale[1]/2, 0, 2*pi)
-    end
-
+function draw_marker(ctx, ::Type{<: Circle}, pos, strokecolor, strokewidth, mat)
+    # There are already active transforms so we can't Cairo.set_matrix() here
+    Cairo.translate(ctx, pos[1], pos[2])
+    cairo_transform(ctx, mat)
+    Cairo.arc(ctx, 0, 0, 0.5, 0, 2*pi)
     Cairo.fill_preserve(ctx)
-
     Cairo.set_line_width(ctx, Float64(strokewidth))
-
     sc = to_color(strokecolor)
     Cairo.set_source_rgba(ctx, rgbatuple(sc)...)
     Cairo.stroke(ctx)
-    scale[1] != scale[2] && Cairo.set_matrix(ctx, old_matrix)
-    nothing
+    return
 end
 
-function draw_marker(ctx, ::Union{Makie.FastPixel,<:Type{<:Rect}}, pos, scale, strokecolor, strokewidth,
-                     marker_offset, rotation)
-
-    Cairo.translate(ctx, pos[1] + marker_offset[1], pos[2] - marker_offset[2])
-    Cairo.rotate(ctx, to_2d_rotation(rotation))
-    Cairo.rectangle(ctx, -0.5scale[1], -0.5scale[2], scale...)
+function draw_marker(ctx, ::Union{Makie.FastPixel,<:Type{<:Rect}}, pos, strokecolor, strokewidth, mat)
+    # There are already active transforms so we can't Cairo.set_matrix() here
+    Cairo.translate(ctx, pos[1], pos[2])
+    cairo_transform(ctx, mat)
+    Cairo.rectangle(ctx, -0.5, -0.5, 1, 1)
     Cairo.fill_preserve(ctx)
     Cairo.set_line_width(ctx, Float64(strokewidth))
     sc = to_color(strokecolor)
     Cairo.set_source_rgba(ctx, rgbatuple(sc)...)
     Cairo.stroke(ctx)
+    return
 end
 
-function draw_marker(ctx, beziermarker::BezierPath, pos, scale, strokecolor, strokewidth, marker_offset, rotation)
+function draw_marker(ctx, beziermarker::BezierPath, pos, strokecolor, strokewidth, mat)
     Cairo.save(ctx)
-    Cairo.translate(ctx, pos[1] + marker_offset[1], pos[2] - marker_offset[2])
-    Cairo.rotate(ctx, to_2d_rotation(rotation))
-    Cairo.scale(ctx, scale[1], -scale[2]) # flip y for cairo
+    # There are already active transforms so we can't Cairo.set_matrix() here
+    Cairo.translate(ctx, pos[1], pos[2])
+    cairo_transform(ctx, mat)
+    Cairo.scale(ctx, 1, -1) # maybe to transition BezierPath y to Cairo y?
     draw_path(ctx, beziermarker)
     Cairo.fill_preserve(ctx)
     sc = to_color(strokecolor)
     Cairo.set_source_rgba(ctx, rgbatuple(sc)...)
     Cairo.set_line_width(ctx, Float64(strokewidth))
     Cairo.stroke(ctx)
     Cairo.restore(ctx)
+    return
 end
 
 function draw_path(ctx, bp::BezierPath)
@@ -495,24 +498,25 @@ function path_command(ctx, c::EllipticalArc)
 end
 
 
-function draw_marker(ctx, marker::Matrix{T}, pos, scale,
-        strokecolor #= unused =#, strokewidth #= unused =#,
-        marker_offset, rotation) where T<:Colorant
+function draw_marker(ctx, marker::Matrix{T}, pos,
+    strokecolor #= unused =#, strokewidth #= unused =#,
+    mat) where T<:Colorant
 
     # convert marker to Cairo compatible image data
     marker = permutedims(marker, (2,1))
     marker_surf = to_cairo_image(marker)
 
     w, h = size(marker)
 
-    Cairo.translate(ctx, pos[1] + marker_offset[1], pos[2] - marker_offset[2])
-    Cairo.rotate(ctx, to_2d_rotation(rotation))
-    Cairo.scale(ctx, scale[1] / w, scale[2] / h)
+    # There are already active transforms so we can't Cairo.set_matrix() here
+    Cairo.translate(ctx, pos[1], pos[2])
+    cairo_transform(ctx, mat)
+    Cairo.scale(ctx, 1.0 / w, 1.0 / h)
     Cairo.set_source_surface(ctx, marker_surf, -w/2, -h/2)
     Cairo.paint(ctx)
+    return
 end
 
-
 ################################################################################
 #                                     Text                                     #
 ################################################################################
@@ -616,28 +620,8 @@ function draw_glyph_collection(
             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
-        # of the character into screen space and then subtracting the projected
-        # origin. The resulting vectors give the directions in which the character
-        # needs to be stretched in order to match the 3D projection
-
-        xvec = rotation * (scale3[1] * Point3d(1, 0, 0))
-        yvec = rotation * (scale3[2] * Point3d(0, -1, 0))
-
-        glyphpos = _project_position(scene, markerspace, gp3, id, true)
-        xproj = _project_position(scene, markerspace, gp3 + model33 * xvec, id, true)
-        yproj = _project_position(scene, markerspace, gp3 + model33 * yvec, id, true)
-
-        xdiff = xproj - glyphpos
-        ydiff = yproj - glyphpos
-
-        mat = Cairo.CairoMatrix(
-            xdiff[1], xdiff[2],
-            ydiff[1], ydiff[2],
-            0, 0,
-        )
+        scale2 = scale isa Number ? Vec2d(scale, scale) : scale
+        glyphpos, mat, _ = project_marker(scene, markerspace, gp3, scale2, rotation, model33, id)
 
         Cairo.save(ctx)
         set_font_matrix(ctx, mat)
@@ -1282,7 +1266,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
     scale = Makie.voxel_size(primitive)
     colors = Makie.voxel_colors(primitive)
     marker = GeometryBasics.expand_faceviews(normal_mesh(Rect3f(Point3f(-0.5), Vec3f(1))))
-    
+
     # transformation to world space
     transformed_pos = _transform_to_world(scene, primitive, pos)
 

CairoMakie/src/utils.jl

@@ -92,22 +92,42 @@ function project_position(@nospecialize(scenelike), space, point, model, yflip::
     project_position(scene, Makie.transform_func(scenelike), space, point, model, yflip)
 end
 
-function project_scale(scene::Scene, space, s::Number, model = Mat4d(I))
-    project_scale(scene, space, Vec2d(s), model)
-end
-
-function project_scale(scene::Scene, space, s, model = Mat4d(I))
-    p4d = model * to_ndim(Vec4d, s, 0)
-    if is_data_space(space)
-        @inbounds p = (scene.camera.projectionview[] * p4d)[Vec(1, 2)]
-        return p .* scene.camera.resolution[] .* 0.5
-    elseif is_pixel_space(space)
-        return p4d[Vec(1, 2)]
-    elseif is_relative_space(space)
-        return p4d[Vec(1, 2)] .* scene.camera.resolution[]
-    else # clip
-        return p4d[Vec(1, 2)] .* scene.camera.resolution[] .* 0.5f0
+function project_marker(scene, markerspace, origin, scale, rotation, model, billboard = false)
+    scale3 = to_ndim(Vec2d, scale, first(scale))
+    model33 = model[Vec(1,2,3), Vec(1,2,3)]
+    origin3 = to_ndim(Point3d, origin, 0)
+    return project_marker(scene, markerspace, origin3, scale3, rotation, model33, Mat4d(I), billboard)
+end
+function project_marker(scene, markerspace, origin::Point3, scale::Vec, rotation, model33::Mat3, id = Mat4d(I), billboard = false)
+    # the CairoMatrix is found by transforming the right and up vector
+    # of the marker into screen space and then subtracting the projected
+    # origin. The resulting vectors give the directions in which the character
+    # needs to be stretched in order to match the 3D projection
+
+    xvec = rotation * (model33 * (scale[1] * Point3d(1, 0, 0)))
+    yvec = rotation * (model33 * (scale[2] * Point3d(0, -1, 0)))
+
+    proj_pos = _project_position(scene, markerspace, origin, id, true)
+
+    if billboard && Makie.is_data_space(markerspace)
+        p4d = scene.camera.view[] * to_ndim(Point4d, origin, 1)
+        xproj = _project_position(scene, :eye, p4d[Vec(1,2,3)] / p4d[4] + xvec, id, true)
+        yproj = _project_position(scene, :eye, p4d[Vec(1,2,3)] / p4d[4] + yvec, id, true)
+    else
+        xproj = _project_position(scene, markerspace, origin + xvec, id, true)
+        yproj = _project_position(scene, markerspace, origin + yvec, id, true)
     end
+
+    xdiff = xproj - proj_pos
+    ydiff = yproj - proj_pos
+
+    mat = Cairo.CairoMatrix(
+        xdiff[1], xdiff[2],
+        ydiff[1], ydiff[2],
+        0, 0,
+    )
+
+    return proj_pos, mat, Mat2f(xdiff..., ydiff...)
 end
 
 function project_shape(@nospecialize(scenelike), space, rect::Rect, model)
@@ -193,16 +213,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