Refactor SurfaceLike trait

GLMakie/src/drawing_primitives.jl

@@ -476,10 +476,9 @@ end
 function draw_atomic(screen::Screen, scene::Scene, x::Image)
     return cached_robj!(screen, scene, x) do gl_attributes
         position = lift(x, x[1], x[2]) do x, y
-            r = to_range(x, y)
-            x, y = minimum(r[1]), minimum(r[2])
-            xmax, ymax = maximum(r[1]), maximum(r[2])
-            rect =  Rect2f(x, y, xmax - x, ymax - y)
+            xmin, xmax = extrema(x)
+            ymin, ymax = extrema(y)
+            rect =  Rect2f(xmin, ymin, xmax - xmin, ymax - ymin)
             return decompose(Point2f, rect)
         end
         gl_attributes[:vertices] = apply_transform(transform_func_obs(x), position, x.space)

MakieCore/src/basic_plots.jl

@@ -121,7 +121,7 @@ calculated_attributes!(plot::T) where T = calculated_attributes!(T, plot)
     image(x, y, image)
     image(image)
 
-Plots an image on range `x, y` (defaults to dimensions).
+Plots an image on a rectangle bounded by `x` and `y` (defaults to size of image).
 
 ## Attributes
 
@@ -146,7 +146,8 @@ end
     heatmap(x, y, values)
     heatmap(values)
 
-Plots a heatmap as an image on `x, y` (defaults to interpretation as dimensions).
+Plots a heatmap as a collection of rectangles centered at `x[i], y[j]` with
+colors derived from `values[i, j]`. (Defaults to `x, y = axes(values)`.)
 
 ## Attributes
 
@@ -215,11 +216,9 @@ end
     surface(x, y, z)
     surface(z)
 
-Plots a surface, where `(x, y)`  define a grid whose heights are the entries in `z`.
+Plots a surface, where `(x, y)` define a grid whose heights are the entries in `z`.
 `x` and `y` may be `Vectors` which define a regular grid, **or** `Matrices` which define an irregular grid.
 
-`Surface` has the conversion trait `ContinuousSurface <: SurfaceLike`.
-
 ## Attributes
 
 ### Specific to `Surface`

MakieCore/src/conversion.jl

@@ -33,17 +33,75 @@ conversion_trait(T::Type, args...) = conversion_trait(T)
 
 convert_arguments(::NoConversion, args...) = args
 
+"""
+    PointBased() <: ConversionTrait
+
+Plots with the `PointBased` trait convert their input data to a
+`Vector{Point{D, Float32}}`.
+"""
 struct PointBased <: ConversionTrait end
 conversion_trait(::Type{<: XYBased}) = PointBased()
 conversion_trait(::Type{<: Text}) = PointBased()
 
-abstract type SurfaceLike <: ConversionTrait end
+"""
+    GridBased <: ConversionTrait
+
+GridBased is an abstract conversion trait for data that exists on a grid.
 
-struct ContinuousSurface <: SurfaceLike end
-conversion_trait(::Type{<: Union{Surface, Image}}) = ContinuousSurface()
+Child types: [`VertexBasedGrid`](@ref), [`CellBasedGrid`](@ref)
+See also: [`ImageLike`](@ref)
+Used for: Scatter, Lines
+"""
+abstract type GridBased <: ConversionTrait end
+
+"""
+    VertexBasedGrid() <: GridBased <: ConversionTrait
+
+Plots with the `VertexBasedGrid` trait convert their input data to
+`(xs::Vector{Float32}, ys::Vector{Float32}, zs::Matrix{Float32})` such that
+`(length(xs), length(ys)) == size(zs)`, or
+`(xs::Matrix{Float32}, ys::Matrix{Float32}, zs::Matrix{Float32})` such that
+`size(xs) == size(ys) == size(zs)`.
+
+See also: [`CellBasedGrid`](@ref), [`ImageLike`](@ref)
+Used for: Surface
+"""
+struct VertexBasedGrid <: GridBased end
+conversion_trait(::Type{<: Surface}) = VertexBasedGrid()
+
+"""
+    CellBasedGrid() <: GridBased <: ConversionTrait
+
+Plots with the `CellBasedGrid` trait convert their input data to
+`(xs::Vector{Float32}, ys::Vector{Float32}, zs::Matrix{Float32})` such that
+`(length(xs), length(ys)) == size(zs) .+ 1`. After the conversion the x and y
+values represent the edges of cells corresponding to z values.
+
+See also: [`VertexBasedGrid`](@ref), [`ImageLike`](@ref)
+Used for: Heatmap
+"""
+struct CellBasedGrid <: GridBased end
+conversion_trait(::Type{<: Heatmap}) = CellBasedGrid()
+
+"""
+    ImageLike() <: ConversionTrait
+
+Plots with the `ImageLike` trait convert their input data to
+`(xs::Interval, ys::Interval, zs::Matrix{Float32})` where xs and ys mark the
+limits of a quad containing zs.
+
+See also: [`CellBasedGrid`](@ref), [`VertexBasedGrid`](@ref)
+Used for: Image
+"""
+struct ImageLike <: ConversionTrait end
+conversion_trait(::Type{<: Image}) = ImageLike()
+# Rect2f(xmin, ymin, xmax, ymax)
 
-struct DiscreteSurface <: SurfaceLike end
-conversion_trait(::Type{<: Heatmap}) = DiscreteSurface()
+# Deprecations
+function ContinuousSurface()
+    error("ContinuousSurface has been deprecated. Use `ImageLike()` or `VertexBasedGrid()` instead.")
+end
+@deprecate DiscreteSurface CellBasedGrid()
 
 struct VolumeLike <: ConversionTrait end
-conversion_trait(::Type{<: Volume}) = VolumeLike()
+conversion_trait(::Type{<: Volume}) = VolumeLike()

NEWS.md

@@ -7,11 +7,12 @@
   [#2544](https://github.com/MakieOrg/Makie.jl/pull/2544)
 - Fixed an issue where NaN was interpreted as zero when rendering `surface` through CairoMakie. [#2598](https://github.com/MakieOrg/Makie.jl/pull/2598)
 - Improved 3D camera handling, hotkeys and functionality [#2746](https://github.com/MakieOrg/Makie.jl/pull/2746)
+- Refactored the `SurfaceLike` family of traits into `VertexBasedGrid`, `CellBasedGrid` and `ImageLike`. [#3106](https://github.com/MakieOrg/Makie.jl/pull/3106)
 
 ## master
 
 - Fixed incorrect placement of contourlabels with transform functions [#3083](https://github.com/MakieOrg/Makie.jl/pull/3083)
-- Fix automatic normal generation for meshes with shading and no normals [#3041](https://github.com/MakieOrg/Makie.jl/pull/3041).
+- Fixed automatic normal generation for meshes with shading and no normals [#3041](https://github.com/MakieOrg/Makie.jl/pull/3041).
 
 ## v0.19.7
 

docs/tutorials/basic-tutorial.md

@@ -221,7 +221,7 @@ lines([Point(0, 0), Point(5, 10), Point(10, 5)])
 
 The input arguments you can use with `lines` and `scatter` are mostly the same because they have the same conversion trait `PointBased`.
 Other plotting functions have different conversion traits, \myreflink{heatmap} for example expects two-dimensional grid data.
-The respective trait is called `DiscreteSurface`.
+The respective trait is called `CellBasedGrid`.
 
 ## Layering multiple plots
 

src/Makie.jl

@@ -73,7 +73,7 @@ using Observables: listeners, to_value, notify
 
 using MakieCore: SceneLike, MakieScreen, ScenePlot, AbstractScene, AbstractPlot, Transformable, Attributes, Combined, Theme, Plot
 using MakieCore: Arrows, Heatmap, Image, Lines, LineSegments, Mesh, MeshScatter, Poly, Scatter, Surface, Text, Volume, Wireframe
-using MakieCore: ConversionTrait, NoConversion, PointBased, SurfaceLike, ContinuousSurface, DiscreteSurface, VolumeLike
+using MakieCore: ConversionTrait, NoConversion, PointBased, GridBased, VertexBasedGrid, CellBasedGrid, ImageLike, VolumeLike
 using MakieCore: Key, @key_str, Automatic, automatic, @recipe
 using MakieCore: Pixel, px, Unit, Billboard
 using MakieCore: not_implemented_for
@@ -85,7 +85,7 @@ import MakieCore: arrows!, heatmap!, image!, lines!, linesegments!, mesh!, meshs
 import MakieCore: convert_arguments, convert_attribute, default_theme, conversion_trait
 
 export @L_str, @colorant_str
-export ConversionTrait, NoConversion, PointBased, SurfaceLike, ContinuousSurface, DiscreteSurface, VolumeLike
+export ConversionTrait, NoConversion, PointBased, GridBased, VertexBasedGrid, CellBasedGrid, ImageLike, VolumeLike
 export Pixel, px, Unit, plotkey, attributes, used_attributes
 
 const RealVector{T} = AbstractVector{T} where T <: Number

src/basic_recipes/contourf.jl

@@ -58,7 +58,7 @@ function _get_isoband_levels(levels::AbstractVector{<:Real}, mi, ma)
     edges
 end
 
-conversion_trait(::Type{<:Contourf}) = ContinuousSurface()
+conversion_trait(::Type{<:Contourf}) = VertexBasedGrid()
 
 function _get_isoband_levels(::Val{:normal}, levels, values)
     _get_isoband_levels(levels, extrema_nan(values)...)

src/basic_recipes/contours.jl

@@ -110,8 +110,8 @@ function to_levels(n::Integer, cnorm)
     range(zmin + dz; step = dz, length = n)
 end
 
-conversion_trait(::Type{<: Contour3d}) = ContinuousSurface()
-conversion_trait(::Type{<: Contour}) = ContinuousSurface()
+conversion_trait(::Type{<: Contour3d}) = VertexBasedGrid()
+conversion_trait(::Type{<: Contour}) = VertexBasedGrid()
 conversion_trait(::Type{<:Contour}, x, y, z, ::Union{Function, AbstractArray{<: Number, 3}}) = VolumeLike()
 conversion_trait(::Type{<: Contour}, ::AbstractArray{<: Number, 3}) = VolumeLike()
 

src/conversions.jl

@@ -302,7 +302,7 @@ end
 
 
 ################################################################################
-#                                 SurfaceLike                                  #
+#                                  GridBased                                   #
 ################################################################################
 
 function edges(v::AbstractVector)
@@ -320,62 +320,63 @@ function edges(v::AbstractVector)
     end
 end
 
-function adjust_axes(::DiscreteSurface, x::AbstractVector{<:Number}, y::AbstractVector{<:Number}, z::AbstractMatrix)
+function adjust_axes(::CellBasedGrid, x::AbstractVector{<:Number}, y::AbstractVector{<:Number}, z::AbstractMatrix)
     x̂, ŷ = map((x, y), size(z)) do v, sz
         return length(v) == sz ? edges(v) : v
     end
     return x̂, ŷ, z
 end
 
-adjust_axes(::SurfaceLike, x, y, z) = x, y, z
+adjust_axes(::VertexBasedGrid, x, y, z) = x, y, z
 
 """
-    convert_arguments(SL::SurfaceLike, x::VecOrMat, y::VecOrMat, z::Matrix)
+    convert_arguments(ct::GridBased, x::VecOrMat, y::VecOrMat, z::Matrix)
 
-If `SL` is `Heatmap` and `x` and `y` are vectors, infer from length of `x` and `y`
+If `ct` is `Heatmap` and `x` and `y` are vectors, infer from length of `x` and `y`
 whether they represent edges or centers of the heatmap bins.
 If they are centers, convert to edges. Convert eltypes to `Float32` and return
 outputs as a `Tuple`.
 """
-function convert_arguments(SL::SurfaceLike, x::AbstractVecOrMat{<: Number}, y::AbstractVecOrMat{<: Number}, z::AbstractMatrix{<: Union{Number, Colorant}})
-    return map(el32convert, adjust_axes(SL, x, y, z))
+function convert_arguments(ct::GridBased, x::AbstractVecOrMat{<: Number}, y::AbstractVecOrMat{<: Number}, z::AbstractMatrix{<: Union{Number, Colorant}})
+    return map(el32convert, adjust_axes(ct, x, y, z))
 end
-function convert_arguments(SL::SurfaceLike, x::AbstractVecOrMat{<: Number}, y::AbstractVecOrMat{<: Number}, z::AbstractMatrix{<:Number})
-    return map(el32convert, adjust_axes(SL, x, y, z))
+function convert_arguments(ct::GridBased, x::AbstractVecOrMat{<: Number}, y::AbstractVecOrMat{<: Number}, z::AbstractMatrix{<:Number})
+    return map(el32convert, adjust_axes(ct, x, y, z))
 end
 
-convert_arguments(sl::SurfaceLike, x::AbstractMatrix, y::AbstractMatrix) = convert_arguments(sl, x, y, zeros(size(y)))
+convert_arguments(ct::VertexBasedGrid, x::AbstractMatrix, y::AbstractMatrix) = convert_arguments(ct, x, y, zeros(size(y)))
 
 """
-    convert_arguments(P, x, y, z)::Tuple{ClosedInterval, ClosedInterval, Matrix}
+    convert_arguments(P, x::RangeLike, y::RangeLike, z::AbstractMatrix)
 
-Takes 2 ClosedIntervals's `x`, `y`, and an AbstractMatrix `z`, and converts the closed range to
-linspaces with size(z, 1/2)
-`P` is the plot Type (it is optional).
+Takes one or two ClosedIntervals `x` and `y` and converts them to closed ranges
+with size(z, 1/2).
 """
-function convert_arguments(P::SurfaceLike, x::ClosedInterval, y::ClosedInterval, z::AbstractMatrix)
+function convert_arguments(P::GridBased, x::RangeLike, y::RangeLike, z::AbstractMatrix)
     convert_arguments(P, to_linspace(x, size(z, 1)), to_linspace(y, size(z, 2)), z)
 end
 
 """
-    convert_arguments(P, Matrix)::Tuple{ClosedInterval, ClosedInterval, Matrix}
-
-Takes an `AbstractMatrix`, converts the dimesions `n` and `m` into `ClosedInterval`,
-and stores the `ClosedInterval` to `n` and `m`, plus the original matrix in a Tuple.
+    convert_arguments(::ImageLike, mat::AbstractMatrix)
 
-`P` is the plot Type (it is optional).
+Generates `ClosedInterval`s of size `0 .. size(mat, 1/2)` as x and y values.
 """
-function convert_arguments(sl::SurfaceLike, data::AbstractMatrix)
+function convert_arguments(::ImageLike, data::AbstractMatrix)
     n, m = Float32.(size(data))
-    convert_arguments(sl, 0f0 .. n, 0f0 .. m, el32convert(data))
+    return (0f0 .. n, 0f0 .. m, el32convert(data))
+end
+function convert_arguments(::ImageLike, xs::RangeLike, ys::RangeLike, data::AbstractMatrix)
+    x = Float32(minimum(xs)) .. Float32(maximum(xs))
+    y = Float32(minimum(ys)) .. Float32(maximum(ys))
+    return (x, y, el32convert(data))
 end
 
-function convert_arguments(ds::DiscreteSurface, data::AbstractMatrix)
+function convert_arguments(ct::GridBased, data::AbstractMatrix)
     n, m = Float32.(size(data))
-    convert_arguments(ds, edges(1:n), edges(1:m), el32convert(data))
+    convert_arguments(ct, 1f0 .. n, 1f0 .. m, el32convert(data))
 end
 
-function convert_arguments(SL::SurfaceLike, x::AbstractVector{<:Number}, y::AbstractVector{<:Number}, z::AbstractVector{<:Number})
+function convert_arguments(ct::GridBased, x::AbstractVector{<:Number}, y::AbstractVector{<:Number}, z::AbstractVector{<:Number})
     if !(length(x) == length(y) == length(z))
         error("x, y and z need to have the same length. Lengths are $(length.((x, y, z)))")
     end
@@ -397,10 +398,27 @@ function convert_arguments(SL::SurfaceLike, x::AbstractVector{<:Number}, y::Abst
         j = searchsortedfirst(y_centers, yi)
         @inbounds zs[i, j] = zi
     end
-    convert_arguments(SL, x_centers, y_centers, zs)
+    convert_arguments(ct, x_centers, y_centers, zs)
 end
 
 
+"""
+    convert_arguments(P, x, y, f)::(Vector, Vector, Matrix)
+
+Takes vectors `x` and `y` and the function `f`, and applies `f` on the grid that `x` and `y` span.
+This is equivalent to `f.(x, y')`.
+`P` is the plot Type (it is optional).
+"""
+function convert_arguments(ct::Union{GridBased, ImageLike}, x::AbstractVector{T1}, y::AbstractVector{T2}, f::Function) where {T1, T2}
+    if !applicable(f, x[1], y[1])
+        error("You need to pass a function with signature f(x::$T1, y::$T2). Found: $f")
+    end
+    T = typeof(f(x[1], y[1]))
+    z = similar(x, T, (length(x), length(y)))
+    z .= f.(x, y')
+    return convert_arguments(ct, x, y, z)
+end
+
 ################################################################################
 #                                  VolumeLike                                  #
 ################################################################################
@@ -618,21 +636,6 @@ function convert_arguments(::VolumeLike, x::AbstractVector, y::AbstractVector, z
     return (x, y, z, el32convert.(f.(_x, _y, _z)))
 end
 
-"""
-    convert_arguments(P, x, y, f)::(Vector, Vector, Matrix)
-
-Takes vectors `x` and `y` and the function `f`, and applies `f` on the grid that `x` and `y` span.
-This is equivalent to `f.(x, y')`.
-`P` is the plot Type (it is optional).
-"""
-function convert_arguments(sl::SurfaceLike, x::AbstractVector{T1}, y::AbstractVector{T2},
-                           f::Function) where {T1,T2}
-    if !applicable(f, x[1], y[1])
-        error("You need to pass a function with signature f(x::$T1, y::$T2). Found: $f")
-    end
-    return convert_arguments(sl, x, y, f.(x, y'))
-end
-
 function convert_arguments(P::PlotFunc, r::AbstractVector, f::Function)
     return convert_arguments(P, r, map(f, r))
 end
@@ -668,7 +671,7 @@ end
 
 
 # OffsetArrays conversions
-function convert_arguments(sl::SurfaceLike, wm::OffsetArray)
+function convert_arguments(sl::GridBased, wm::OffsetArray)
   x1, y1 = wm.offsets .+ 1
   nx, ny = size(wm)
   x = range(x1, length = nx)

test/boundingboxes.jl

@@ -10,8 +10,8 @@
 
     fig, ax, p = surface([x*y for x in 1:10, y in 1:10])
     bb = boundingbox(p)
-    @test bb.origin ≈ Point3f(0.0, 0.0, 1.0)
-    @test bb.widths ≈ Vec3f(10.0, 10.0, 99.0)
+    @test bb.origin ≈ Point3f(1.0, 1.0, 1.0)
+    @test bb.widths ≈ Vec3f(9.0, 9.0, 99.0)
 
     fig, ax, p = meshscatter([Point3f(x, y, z) for x in 1:5 for y in 1:5 for z in 1:5])
     bb = boundingbox(p)
@@ -42,7 +42,7 @@
     bb = boundingbox(p)
     @test bb.origin ≈ Point3f(0.5, 0.5, 0)
     @test bb.widths ≈ Vec3f(10.0, 10.0, 0)
-    
+
     fig, ax, p = image(rand(10, 10))
     bb = boundingbox(p)
     @test bb.origin ≈ Point3f(0)