Improve model and transform_func interaction with Axis limits

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## [Unreleased]
 
+- `boundingbox` now relies on `apply_transform(transform, data_limits(plot))` rather than transforming the corner points of the bounding box [#3856](https://github.com/MakieOrg/Makie.jl/pull/3856)
+- Adjusted `Axis` limits to consider transformations more consistently [#3864](https://github.com/MakieOrg/Makie.jl/pull/3864)
+
 ## [0.21.0] - 2024-05-08
 
 - Add `voxels` plot [#3527](https://github.com/MakieOrg/Makie.jl/pull/3527).
@@ -14,7 +17,7 @@
   - `data_limits` now only considers plot positions, completely ignoring transformations
   - `boundingbox(p::Text)` is deprecated in favor of `boundingbox(p::Text, p.markerspace[])`. The more internal methods use `string_boundingbox(p)`. [#3723](https://github.com/MakieOrg/Makie.jl/pull/3723)
   - `boundingbox` overwrites must now include a secondary space argument to work `boundingbox(plot, space::Symbol = :data)` [#3723](https://github.com/MakieOrg/Makie.jl/pull/3723)
-  - `boundingbox` now always consider `transform_func` and `model` (except for Text for the time being)
+  - `boundingbox` now always consider `transform_func` and `model`
   - `data_limits(::Scatter)` and `boundingbox(::Scatter)` now consider marker transformations [#3716](https://github.com/MakieOrg/Makie.jl/pull/3716)
 - **Breaking** Improved Float64 compatability of Axis [#3681](https://github.com/MakieOrg/Makie.jl/pull/3681)
   - This added an extra conversion step which only takes effect when Float32 precision becomes relevant. In those cases code using `project()` functions will be wrong as the transformation is not applied. Use `project(plot_or_scene, ...)` or apply the conversion yourself beforehand with `Makie.f32_convert(plot_or_scene, transformed_point)` and use `patched_model = Makie.patch_model(plot_or_scene, model)`.

ReferenceTests/src/tests/examples2d.jl

@@ -1151,10 +1151,10 @@ end
 
     fig = Figure(size = (600, 600))
     # Create a recipe plot
-    ax, plot_top = heatmap(fig[1, 1], randn(10, 10))
+    ax, plot_top = heatmap(fig[1, 1], randn(10, 10), colormap = [:transparent])
     # Plot some recipes at the level below the contour
-    scatterlineplot_1 = scatterlines!(plot_top, 1:10, 1:10; linewidth = 20, markersize = 20, color = :red)
-    scatterlineplot_2 = scatterlines!(plot_top, 1:10, 1:10; linewidth = 20, markersize = 30, color = :blue)
+    scatterlineplot_1 = scatterlines!(ax, 1:10, 1:10; linewidth = 20, markersize = 20, color = :red)
+    scatterlineplot_2 = scatterlines!(ax, 1:10, 1:10; linewidth = 20, markersize = 30, color = :blue)
     # Translate the lowest level plots (scatters)
     translate!(scatterlineplot_1.plots[2], 0, 0, 1)
     translate!(scatterlineplot_2.plots[2], 0, 0, -1)

ReferenceTests/src/tests/figures_and_makielayout.jl

@@ -327,3 +327,16 @@ end
     fig
 end
 
+@reference_test "Axis limits with translation, scaling and transform_func" begin
+    f = Figure()
+    a = Axis(f[1,1], xscale = log10, yscale = log10)
+    ps = Point2f.([0.1, 0.1, 1000, 1000], [1, 100, 1, 100])
+    hl = linesegments!(a, ps[[1, 3, 2, 4]], color = :red)
+    vl = linesegments!(a, ps, color = :blue)
+    # translation happens before scale! here because scale! acts on scene and
+    # translate! acts on the plot (these are combined by matmult)
+    scale!(a.scene, 0.5, 2, 1.0)
+    translate!(hl, 0, 1, 0)
+    translate!(vl, 1, 0, 0)
+    f
+end

src/basic_recipes/bracket.jl

@@ -122,7 +122,7 @@ function plot!(pl::Bracket)
 end
 
 data_limits(pl::Bracket) = mapreduce(ps -> Rect3d([ps...]), union, pl[1][])
-boundingbox(pl::Bracket, space::Symbol = :data) = transform_bbox(pl, data_limits(pl))
+boundingbox(pl::Bracket, space::Symbol = :data) = apply_transform_and_model(pl, data_limits(pl))
 
 bracket_bezierpath(style::Symbol, args...) = bracket_bezierpath(Val(style), args...)
 

src/basic_recipes/contours.jl

@@ -352,9 +352,9 @@ function data_limits(plot::Contour{<: Tuple{X, Y, Z}}) where {X, Y, Z}
     return Rect3d(mini, maxi .- mini)
 end
 function boundingbox(plot::Contour{<: Tuple{X, Y, Z}}, space::Symbol = :data) where {X, Y, Z}
-    return transform_bbox(plot, data_limits(plot))
+    return apply_transform_and_model(plot, data_limits(plot))
 end
 # TODO: should this have a data_limits overload?
 function boundingbox(plot::Contour3d, space::Symbol = :data)
-    return transform_bbox(plot, data_limits(plot))
+    return apply_transform_and_model(plot, data_limits(plot))
 end

src/basic_recipes/datashader.jl

@@ -469,7 +469,7 @@ function Makie.plot!(p::DataShader{<:Tuple{Dict{String, Vector{Point{2, Float32}
 end
 
 data_limits(p::DataShader) =  p._boundingbox[]
-boundingbox(p::DataShader, space::Symbol = :data) =  transform_bbox(p, p._boundingbox[])
+boundingbox(p::DataShader, space::Symbol = :data) = apply_transform_and_model(p, p._boundingbox[])
 
 function convert_arguments(P::Type{<:Union{MeshScatter,Image,Surface,Contour,Contour3d}}, canvas::Canvas, operation=automatic, local_operation=identity)
     pixel = Aggregation.get_aggregation(canvas; operation=operation, local_operation=local_operation)

src/basic_recipes/error_and_rangebars.jl

@@ -304,4 +304,4 @@ end
 
 # ignore whiskers when determining data limits
 data_limits(bars::Union{Errorbars, Rangebars}) = data_limits(bars.plots[1])
-boundingbox(bars::Union{Errorbars, Rangebars}, space::Symbol = :data) = transform_bbox(bars, data_limits(bars))
+boundingbox(bars::Union{Errorbars, Rangebars}, space::Symbol = :data) = apply_transform_and_model(bars, data_limits(bars))

src/basic_recipes/hvlines.jl

@@ -95,4 +95,4 @@ function data_limits(p::VLines)
     return Rect3d(Point3d(xmin, NaN, 0), Vec3d(xmax - xmin, NaN, 0))
 end
 
-boundingbox(p::Union{HLines, VLines}, space::Symbol = :data) = transform_bbox(p, data_limits(p))
+boundingbox(p::Union{HLines, VLines}, space::Symbol = :data) = apply_transform_and_model(p, data_limits(p))

src/basic_recipes/hvspan.jl

@@ -99,6 +99,6 @@ function data_limits(p::VSpan)
     return Rect3d(Point3d(xmin, NaN, 0), Vec3d(xmax - xmin, NaN, 0))
 end
 
-boundingbox(p::Union{HSpan, VSpan}, space::Symbol = :data) = transform_bbox(p, data_limits(p))
+boundingbox(p::Union{HSpan, VSpan}, space::Symbol = :data) = apply_transform_and_model(p, data_limits(p))
 
 convert_arguments(P::Type{<:Union{HSpan, VSpan}}, x::Interval) = convert_arguments(P, endpoints(x)...)

src/basic_recipes/triplot.jl

@@ -248,4 +248,4 @@ function data_limits(p::Triplot{<:Tuple{<:Vector{<:Point}}})
         return data_limits(p.plots[1])
     end
 end
-boundingbox(p::Triplot{<:Tuple{<:Vector{<:Point}}}, space::Symbol = :data) = transform_bbox(p, data_limits(p))
+boundingbox(p::Triplot{<:Tuple{<:Vector{<:Point}}}, space::Symbol = :data) = apply_transform_and_model(p, data_limits(p))

src/basic_recipes/voronoiplot.jl

@@ -166,7 +166,7 @@ function data_limits(p::Voronoiplot{<:Tuple{<:Vector{<:Point}}})
         return data_limits(p.plots[1])
     end
 end
-boundingbox(p::Voronoiplot{<:Tuple{<:Vector{<:Point}}}, space::Symbol = :data) = transform_bbox(p, data_limits(p))
+boundingbox(p::Voronoiplot{<:Tuple{<:Vector{<:Point}}}, space::Symbol = :data) = apply_transform_and_model(p, data_limits(p))
 
 function plot!(p::Voronoiplot{<:Tuple{<:DelTri.VoronoiTessellation}})
     generators_2f = Observable(Point2f[])

src/camera/projection_math.jl

@@ -222,6 +222,11 @@ function transformationmatrix(translation, scale, rotation::Quaternion)
     trans_scale*rotation
 end
 
+function is_translation_scale_matrix(mat::Mat4{T}) where T
+    return abs(mat[2, 1]) + abs(mat[3, 1]) + abs(mat[1, 2]) + abs(mat[3, 2]) +
+           abs(mat[1, 3]) + abs(mat[2, 3]) < sqrt(eps(T))
+end
+
 #Calculate rotation between two vectors
 function rotation(u::Vec{3, T}, v::Vec{3, T}) where T
     # It is important that the inputs are of equal length when

src/layouting/boundingbox.jl

@@ -35,7 +35,7 @@ See also: [`data_limits`](@ref)
 function boundingbox(plot::AbstractPlot, space::Symbol = :data)
     # Assume primitive plot
     if isempty(plot.plots)
-        return Rect3d(iterate_transformed(plot))
+        return apply_transform_and_model(plot, data_limits(plot))
     end
 
     # Assume combined plot
@@ -47,11 +47,6 @@ function boundingbox(plot::AbstractPlot, space::Symbol = :data)
     return bb_ref[]
 end
 
-# for convenience
-function transform_bbox(scenelike, lims::Rect)
-    return Rect3d(iterate_transformed(scenelike, point_iterator(lims)))
-end
-
 # same as data_limits except using iterate_transformed
 function boundingbox(plot::MeshScatter, space::Symbol = :data)
     # TODO: avoid mesh generation here if possible
@@ -111,7 +106,7 @@ function boundingbox(plot::Scatter)
         return bb
 
     else
-        return Rect3d(iterate_transformed(plot))
+        return apply_transform_and_model(plot, data_limits(plot))
     end
 end
 
@@ -127,12 +122,3 @@ end
 function iterate_transformed(plot, points::AbstractArray{<: VecTypes})
     return apply_transform_and_model(plot, points)
 end
-
-# TODO: Can this be deleted?
-function iterate_transformed(plot, points::T) where T
-    @warn "iterate_transformed with $T"
-    t = transformation(plot)
-    model = model_transform(t) # will auto-promote if points if Float64
-    trans_func = transform_func(t)
-    [to_ndim(Point3d, project(model, apply_transform(trans_func, point, space))) for point in points]
-end

src/layouting/text_boundingbox.jl

@@ -9,12 +9,8 @@ function boundingbox(plot::Text, target_space::Symbol)
     # transformations (i.e. drops textsize etc when markerspace is not part of
     # the plot.space -> target_space conversion chain)
     if target_space == :data
-        if plot.space[] == plot.markerspace[]
-            # probably shouldn't transform...
-            return transform_bbox(plot, string_boundingbox(plot))
-        else
-            return Rect3d(iterate_transformed(plot))
-        end
+        # This also transforms string boundingboxes if space == markerspace
+        return apply_transform_and_model(plot, data_limits(plot))
     elseif target_space == plot.markerspace[]
         return string_boundingbox(plot)
     else

src/layouting/transformation.jl

@@ -1,3 +1,11 @@
+#=
+`apply_transform` Interface for custom transformations:
+- can be a struct holding information about the transformation or a function
+- should implement `inverse_transform(transformation)`
+- for struct: must implement `apply_transform(transform, ::VecTypes)` (for 2d and 3d points)
+- for struct: must implement `apply_transform(transform, ::Rect3d)` for bounding boxes
+=#
+
 Base.parent(t::Transformation) = isassigned(t.parent) ? t.parent[] : nothing
 
 function parent_transform(x)
@@ -183,37 +191,75 @@ transform_func(x) = transform_func_obs(x)[]
 transform_func_obs(x) = transformation(x).transform_func
 
 """
-    apply_transform_and_model(plot, pos, output_type = Point3d)
-    apply_transform_and_model(model, transfrom_func, pos, output_type = Point3d)
+    apply_transform_and_model(plot, data, output_type = Point3d)
+    apply_transform_and_model(model, transfrom_func, data, output_type = Point3d)
 
 Applies the transform function and model matrix (i.e. transformations from
-`translate!`, `rotate!` and `scale!`) to the given input
+`translate!`, `rotate!` and `scale!`) to the given input.
 """
-function apply_transform_and_model(plot::AbstractPlot, pos, output_type = Point3d)
+function apply_transform_and_model(plot::AbstractPlot, data, output_type = Point3d)
     return apply_transform_and_model(
-        plot.model[], transform_func(plot), pos,
+        plot.model[], transform_func(plot), data,
         to_value(get(plot, :space, :data)),
         output_type
     )
 end
-function apply_transform_and_model(model::Mat4, f, pos::VecTypes, space = :data, output_type = Point3d)
-    transformed = apply_transform(f, pos, space)
+function apply_transform_and_model(model::Mat4, f, data, space = :data, output_type = Point3d)
+    transformed = apply_transform(f, data, space)
+    world = apply_model(model, transformed, space)
+    return promote_geom(output_type, world)
+end
+
+function unchecked_apply_model(model::Mat4, transformed::VecTypes)
+    p4d = to_ndim(Point4d, to_ndim(Point3d, transformed, 0), 1)
+    p4d = model * p4d
+    p4d = p4d ./ p4d[4]
+    return p4d
+end
+@inline function apply_model(model::Mat4, transformed::AbstractArray, space::Symbol)
+    if space in (:data, :transformed)
+        return unchecked_apply_model.((model,), transformed)
+    else
+        return transformed
+    end
+end
+function apply_model(model::Mat4, transformed::VecTypes, space::Symbol)
     if space in (:data, :transformed)
-        p4d = to_ndim(Point4d, to_ndim(Point3d, transformed, 0), 1)
-        p4d = model * p4d
-        p4d = p4d ./ p4d[4]
-        return to_ndim(output_type, p4d, NaN)
+        return unchecked_apply_model(model, transformed)
     else
-        return to_ndim(output_type, transformed, NaN)
+        return transformed
     end
 end
-function apply_transform_and_model(model::Mat4, f, positions::AbstractArray, space = :data, output_type = Point3d)
-    output = similar(positions, output_type)
-    return map!(output, positions) do pos
-        apply_transform_and_model(model, f, pos, space, output_type)
+function apply_model(model::Mat4, transformed::Rect{N, T}, space::Symbol) where {N, T}
+    if space in (:data, :transformed)
+        bb = Rect{N, T}()
+        if !isfinite_rect(transformed) && is_translation_scale_matrix(model)
+            # Bbox contains NaN so matmult would make everything NaN, but model
+            # matrix doesn't actually mix dimensions (just translation + scaling)
+            scale = to_ndim(Vec{N, T}, Vec3(model[1, 1], model[2, 2], model[3, 3]), 1.0)
+            trans = to_ndim(Vec{N, T}, Vec3(model[1, 4], model[2, 4], model[3, 4]), 1.0)
+            mini = scale .* minimum(transformed) .+ trans
+            maxi = scale .* maximum(transformed) .+ trans
+            return Rect{N, T}(mini, maxi - mini)
+        else
+            for input in corners(transformed)
+                output = to_ndim(Point{N, T}, unchecked_apply_model(model, input), NaN)
+                bb = update_boundingbox(bb, output)
+            end
+        end
+        return bb
+    else
+        return transformed
     end
 end
 
+promote_geom(output_type::Type{<:VecTypes}, x::VecTypes) = to_ndim(output_type, x, NaN)
+promote_geom(output_type::Type{<:VecTypes}, x::AbstractArray) = promote_geom.(output_type, x)
+function promote_geom(output_type::Type{<:VecTypes}, x::T) where {T <: Rect}
+    return T(promote_geom(output_type, minimum(x)), promote_geom(output_type, widths(x)))
+end
+
+
 """
     apply_transform(f, data, space)
 Apply the data transform func to the data if the space matches one
@@ -247,7 +293,6 @@ apply_transform(f::NTuple{3, typeof(identity)}, x::VecTypes) = x
 apply_transform(f::NTuple{3, typeof(identity)}, x::Number) = x
 apply_transform(f::NTuple{3, typeof(identity)}, x::ClosedInterval) = x
 
-
 struct PointTrans{N, F}
     f::F
     function PointTrans{N}(f::F) where {N, F}
@@ -449,6 +494,20 @@ function apply_transform(f::Polar, point::VecTypes{N2, T}) where {N2, T}
     end
 end
 
+# For bbox
+function apply_transform(trans::Polar, bbox::Rect3d)
+    if trans.theta_as_x
+        θmin, rmin, zmin = minimum(bbox)
+        θmax, rmax, zmax = maximum(bbox)
+    else
+        rmin, θmin, zmin = minimum(bbox)
+        rmax, θmax, zmax = maximum(bbox)
+    end
+    bb2d = polaraxis_bbox((rmin, rmax), (θmin, θmax), trans.r0, trans.direction, trans.theta_0)
+    o = minimum(bb2d); w = widths(bb2d)
+    return Rect3d(to_ndim(Point3d, o, zmin), to_ndim(Vec3d, w, zmax - zmin))
+end
+
 function inverse_transform(trans::Polar)
     if trans.theta_as_x
         return Makie.PointTrans{2}() do point

src/makielayout/blocks/axis.jl

@@ -80,7 +80,6 @@ function update_axis_camera(scene::Scene, t, lims, xrev::Bool, yrev::Bool)
     tlims = Makie.apply_transform(t, lims)
     camera = scene.camera
 
-    # TODO: apply model
     update_limits!(scene.float32convert, tlims) # update float32 scaling
     lims32 = f32_convert(scene.float32convert, tlims)  # get scaled limits
     left, bottom = minimum(lims32)
@@ -853,33 +852,31 @@ function getlimits(la::Axis, dim)
         return !to_value(get(plot, :visible, true))
     end
 
-    # TODO:
-    # We used to include scale! and rotate! in data_limits. For compat we include
-    # them here again until we implement a full solution
-
-    # # get all data limits, minus the excluded plots
-    # boundingbox = Makie.data_limits(la.scene, exclude)
-    bb_ref = Base.RefValue(Rect3d())
-    for plot in la.scene
-        if !exclude(plot)
-            bb = data_limits(plot)
-            # Limits can be one dimensional (partially NaN) e.g. for hlines
-            # which results in every model * point to become NaN. For now we skip
-            # model application if the model matrix is identity to avoid this...
-            model = plot.model[][Vec(1,2,3), Vec(1,2,3)]
-            if !(model ≈ I)
-                bb = Rect3d(map(p -> model * to_ndim(Point3d, p, 0), coordinates(bb)))
-            end
-            update_boundingbox!(bb_ref, bb)
+    # get all data limits, minus the excluded plots
+    tf = la.scene.transformation.transform_func[]
+    itf = inverse_transform(tf)
+    if itf === nothing
+        @warn "Axis transformation $tf does not define an `inverse_transform()`. This may result in a bad choice of limits due to model transformations being ignored." maxlog = 1
+        bb = data_limits(la.scene, exclude)
+    else
+        # get limits with transform_func and model applied
+        bb = boundingbox(la.scene, exclude)
+        # then undo transform_func so that ticks can handle transform_func
+        # without ignoring translations, scaling or rotations from model
+        try
+            bb = apply_transform(itf, bb)
+        catch e
+            # TODO: Is this necessary?
+            @warn "Failed to apply inverse transform $itf to bounding box $bb. Falling back on data_limits()." exception = e
+            bb = data_limits(la.scene, exclude)
         end
     end
-    boundingbox = bb_ref[]
 
     # if there are no bboxes remaining, `nothing` signals that no limits could be determined
-    isfinite_rect(boundingbox, dim) || return nothing
+    isfinite_rect(bb, dim) || return nothing
 
     # otherwise start with the first box
-    mini, maxi = minimum(boundingbox), maximum(boundingbox)
+    mini, maxi = minimum(bb), maximum(bb)
     return (mini[dim], maxi[dim])
 end