Deprecate rotations for rotation for Scatter and MeshScatter (#3724)

* deprecate `rotations` for `rotation` for Scatter and MeshScatter

* one more rename

* remove double conversion

* fix cairomakie meshscatter

* fix usage in docs

* remove mat4 code again

* fix CairoMakie rror

* change one more rotations

* fix WGLMakie

* add changelog entry

---------

Co-authored-by: ffreyer <frederic481994@hotmail.de>

CHANGELOG.md

@@ -26,6 +26,7 @@
   - WGLMakie: Added native anti-aliasing which generally improves quality but introduces outline artifacts in some cases (same as GLMakie)
   - Both: Adjusted handling of thin lines which may result in different color intensities
 - Fixed an issue with lines being drawn in the wrong direction in 3D (with perspective projection) [#3651](https://github.com/MakieOrg/Makie.jl/pull/3651).
+- **Breaking** Renamed attribute `rotations` to `rotation` for `scatter` and `meshscatter` which had been inconsistent with the otherwise singular naming scheme and other plots like `text` [#3724](https://github.com/MakieOrg/Makie.jl/pull/3724).
 
 
 ## [0.20.8] - 2024-02-22

CairoMakie/src/primitives.jl

@@ -369,7 +369,7 @@ end
 ################################################################################
 
 function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scatter))
-    @get_attribute(primitive, (markersize, strokecolor, strokewidth, marker, marker_offset, rotations, transform_marker))
+    @get_attribute(primitive, (markersize, strokecolor, strokewidth, marker, marker_offset, rotation, transform_marker))
 
     ctx = screen.context
     model = primitive.model[]
@@ -386,16 +386,16 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scat
     transfunc = Makie.transform_func(primitive)
 
     return draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker,
-                               marker_offset, rotations, model, positions, size_model, font, markerspace,
+                               marker_offset, rotation, model, positions, size_model, font, markerspace,
                                space)
 end
 
-function draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker, marker_offset, rotations, model, positions, size_model, font, markerspace, space)
+function draw_atomic_scatter(scene, ctx, transfunc, colors, markersize, strokecolor, strokewidth, marker, marker_offset, rotation, model, positions, size_model, font, markerspace, space)
     # TODO Optimization:
     # avoid calling project functions per element as they recalculate the
     # combined projection matrix for each element like this
     broadcast_foreach(positions, colors, markersize, strokecolor,
-            strokewidth, marker, marker_offset, remove_billboard(rotations)) do point, col,
+            strokewidth, marker, marker_offset, remove_billboard(rotation)) do point, col,
             markersize, strokecolor, strokewidth, m, mo, rotation
 
         scale = project_scale(scene, markerspace, markersize, size_model)
@@ -1167,7 +1167,7 @@ end
 
 
 function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Makie.MeshScatter))
-    @get_attribute(primitive, (model, marker, markersize, rotations))
+    @get_attribute(primitive, (model, marker, markersize, rotation))
     pos = primitive[1][]
     # For correct z-ordering we need to be in view/camera or screen space
     model = copy(model)
@@ -1198,16 +1198,16 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
             submesh[:calculated_colors] = color[i]
         end
         scale = markersize isa Vector ? markersize[i] : markersize
-        rotation = if rotations isa Vector
-            Makie.rotationmatrix4(to_rotation(rotations[i]))
+        _rotation = if rotation isa Vector
+            Makie.rotationmatrix4(to_rotation(rotation[i]))
         else
-            Makie.rotationmatrix4(to_rotation(rotations))
+            Makie.rotationmatrix4(to_rotation(rotation))
         end
 
         draw_mesh3D(
             scene, screen, submesh, marker, pos = p,
             scale = scale isa Real ? Vec3f(scale) : to_ndim(Vec3f, scale, 1f0),
-            rotation = rotation
+            rotation = _rotation
         )
     end
 

GLMakie/src/drawing_primitives.jl

@@ -395,7 +395,7 @@ function draw_atomic(screen::Screen, scene::Scene, @nospecialize(plot::Union{Sca
             end
             # fast pixel does its own setup
             if !(marker[] isa FastPixel)
-                gl_attributes[:billboard] = lift(rot -> isa(rot, Billboard), plot, plot.rotations)
+                gl_attributes[:billboard] = lift(rot -> isa(rot, Billboard), plot, plot.rotation)
                 atlas = gl_texture_atlas()
                 isnothing(gl_attributes[:distancefield][]) && delete!(gl_attributes, :distancefield)
                 shape = lift(m -> Cint(Makie.marker_to_sdf_shape(m)), plot, marker)

GLMakie/test/glmakie_refimages.jl

@@ -37,7 +37,7 @@ end
     end
 
     fig, ax, p = meshscatter(pos,
-        rotations=rot,
+        rotation=rot,
         color=color,
         markersize=size,
         axis = (; scenekw = (;limits=Rect3f(Point3(0), Point3(1))))

MakieCore/src/basic_plots.jl

@@ -385,7 +385,7 @@ Plots a marker for each element in `(x, y, z)`, `(x, y)`, or `positions`.
     glowwidth = 0.0
 
     "Sets the rotation of the marker. A `Billboard` rotation is always around the depth axis."
-    rotations = Billboard()
+    rotation = Billboard()
     "The offset of the marker from the given position in `markerspace` units. Default is centered around the position (markersize * -0.5)."
     marker_offset = automatic
     "Controls whether the model matrix (without translation) applies to the marker itself, rather than just the positions. (If this is true, `scale!` and `rotate!` will affect the marker."
@@ -404,6 +404,12 @@ Plots a marker for each element in `(x, y, z)`, `(x, y)`, or `positions`.
     fxaa = false
 end
 
+function deprecated_attributes(::Type{<:Scatter})
+    (
+        (; attribute = :rotations, message = "`rotations` has been renamed to `rotation` for consistency in Makie v0.21.", error = true),
+    )
+end
+
 """
     meshscatter(positions)
     meshscatter(x, y)
@@ -420,13 +426,19 @@ Plots a mesh for each element in `(x, y, z)`, `(x, y)`, or `positions` (similar
     "Sets the scale of the mesh. This can be given as a `Vector` to apply to each scattered mesh individually."
     markersize = 0.1
     "Sets the rotation of the mesh. A numeric rotation is around the z-axis, a `Vec3f` causes the mesh to rotate such that the the z-axis is now that vector, and a quaternion describes a general rotation. This can be given as a Vector to apply to each scattered mesh individually."
-    rotations = 0.0
+    rotation = 0.0
     cycle = [:color]
     mixin_generic_plot_attributes()...
     mixin_shading_attributes()...
     mixin_colormap_attributes()...
 end
 
+function deprecated_attributes(::Type{<:MeshScatter})
+    (
+        (; attribute = :rotations, message = "`rotations` has been renamed to `rotation` for consistency in Makie v0.21.", error = true),
+    )
+end
+
 """
     text(positions; text, kwargs...)
     text(x, y; text, kwargs...)

MakieCore/src/recipes.jl

@@ -692,10 +692,11 @@ function validate_attribute_keys(P::Type{<:Plot}, kw::Dict{Symbol})
     end
     for (deprecated, message, should_error) in deprecations
         if haskey(kw, deprecated)
+            full_message = "Keyword `$deprecated` is deprecated for plot type $P. $message"
             if should_error
-                throw(ArgumentError(message))
+                throw(ArgumentError(full_message))
             else
-                @warn message
+                @warn full_message
             end
         end
     end

RPRMakie/src/meshes.jl

@@ -112,7 +112,7 @@ function to_rpr_object(context, matsys, scene, plot::Makie.MeshScatter)
         markersize
     end
 
-    rotations = Makie.to_rotation(plot.rotations[])
+    rotations = Makie.to_rotation(plot.rotation[])
 
     rotations = if rotations isa Makie.Quaternion
         Iterators.repeated(rotations, n_instances)

ReferenceTests/src/tests/examples3d.jl

@@ -55,7 +55,7 @@ end
     rot = qrotation(Vec3f(1, 0, 0), 0.5pi) * qrotation(Vec3f(0, 1, 0), 0.7pi)
     meshscatter(
         1:3, 1:3, fill(0, 3, 3),
-        marker=catmesh, color=img, markersize=1, rotations=rot,
+        marker=catmesh, color=img, markersize=1, rotation=rot,
         axis=(type=LScene, show_axis=false)
     )
 end
@@ -348,7 +348,7 @@ end
     fig, ax, meshplot = meshscatter(
         pG[edges[:, 1]],
         color=colorsC, marker=meshC,
-        markersize=sizesC,  rotations=rotationsC,
+        markersize=sizesC,  rotation=rotationsC,
     )
     meshscatter!(
         ax, pG,
@@ -373,7 +373,7 @@ end
 @reference_test "image scatter" begin
     scatter(
         1:10, 1:10, RNG.rand(10, 10) .* 10,
-        rotations=normalize.(RNG.rand(Quaternionf, 10 * 10)),
+        rotation=normalize.(RNG.rand(Quaternionf, 10 * 10)),
         markersize=20,
         # can also be an array of images for each point
         # need to be the same size for best performance, though

ReferenceTests/src/tests/primitives.jl

@@ -120,7 +120,7 @@ end
                 p,
                 marker = m,
                 markersize = 30,
-                rotations = rot,
+                rotation = rot,
                 color=:black
             )
             scatter!(s, p, color = :red, markersize = 6)
@@ -209,7 +209,7 @@ end
             p,
             marker = marker,
             markersize = 75,
-            rotations = rot,
+            rotation = rot,
         )
     end
     s

ReferenceTests/src/tests/short_tests.jl

@@ -22,7 +22,7 @@ end
 @reference_test "scatter rotation" begin
     angles = range(0, stop=2pi, length=20)
     pos = Point2f.(sin.(angles), cos.(angles))
-    f, ax, pl = scatter(pos, markersize=0.2, markerspace=:data, rotations=-angles, marker='▲', axis=(;aspect = DataAspect()))
+    f, ax, pl = scatter(pos, markersize=0.2, markerspace=:data, rotation=-angles, marker='▲', axis=(;aspect = DataAspect()))
     scatter!(pos, markersize=10, color=:red)
     f
 end

WGLMakie/assets/particles.vert

@@ -31,7 +31,7 @@ void main(){
     // those functions will get inserted by the shader creation pipeline
     vec3 vertex_position = get_markersize() * to_vec3(get_position());
     vec3 N = get_normals();
-    rotate(get_rotations(), vertex_position, N);
+    rotate(get_rotation(), vertex_position, N);
     vertex_position = to_vec3(get_offset()) + vertex_position;
     vec4 position_world = model * vec4(vertex_position, 1);
     frag_normal = N;

WGLMakie/assets/sprites.vert

@@ -76,7 +76,7 @@ void main(){
     data_point = pview * data_point;
 
     // Compute transform for the offset vectors from the central point
-    trans = (get_billboard() ? projection : pview) * qmat(get_rotations()) * trans;
+    trans = (get_billboard() ? projection : pview) * qmat(get_rotation()) * trans;
     vec4 sprite_center = trans * vec4(sprite_bbox_centre, 0, 0);
 
     vec4 vclip = data_point + sprite_center;

WGLMakie/src/particles.jl

@@ -36,7 +36,7 @@ function handle_color_getter!(uniform_dict, per_instance)
 end
 
 const IGNORE_KEYS = Set([
-    :shading, :overdraw, :rotation, :distancefield, :space, :markerspace, :fxaa,
+    :shading, :overdraw, :distancefield, :space, :markerspace, :fxaa,
     :visible, :transformation, :alpha, :linewidth, :transparency, :marker,
     :light_direction, :light_color,
     :cycle, :label, :inspector_clear, :inspector_hover,
@@ -46,7 +46,7 @@ const IGNORE_KEYS = Set([
 
 function create_shader(scene::Scene, plot::MeshScatter)
     # Potentially per instance attributes
-    per_instance_keys = (:rotations, :markersize, :intensity)
+    per_instance_keys = (:rotation, :markersize, :intensity)
     per_instance = filter(plot.attributes.attributes) do (k, v)
         return k in per_instance_keys && !(isscalar(v[]))
     end
@@ -126,7 +126,7 @@ end
 
 function scatter_shader(scene::Scene, attributes, plot)
     # Potentially per instance attributes
-    per_instance_keys = (:pos, :rotations, :markersize, :color, :intensity,
+    per_instance_keys = (:pos, :rotation, :markersize, :color, :intensity,
                          :uv_offset_width, :quad_offset, :marker_offset)
     uniform_dict = Dict{Symbol,Any}()
     uniform_dict[:image] = false
@@ -225,7 +225,7 @@ function create_shader(scene::Scene, plot::Scatter)
     quad_offset = get(attributes, :marker_offset, Observable(Vec2f(0)))
     attributes[:marker_offset] = Vec3f(0)
     attributes[:quad_offset] = quad_offset
-    attributes[:billboard] = lift(rot -> isa(rot, Billboard), plot, plot.rotations)
+    attributes[:billboard] = lift(rot -> isa(rot, Billboard), plot, plot.rotation)
     attributes[:model] = map(Makie.patch_model, f32_conversion_obs(plot), plot.model)
     attributes[:depth_shift] = get(plot, :depth_shift, Observable(0f0))
 
@@ -280,7 +280,7 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
     uniforms = Dict(
         :model => map(Makie.patch_model, f32_conversion_obs(plot), plot.model),
         :shape_type => Observable(Cint(3)),
-        :rotations => uniform_rotation,
+        :rotation => uniform_rotation,
         :pos => positions,
         :marker_offset => char_offset,
         :quad_offset => quad_offset,

docs/explanations/conversion_pipeline.md

@@ -29,7 +29,7 @@ function myarrows!(scene, ps; kwargs...)
     lines!(scene, ps; kwargs...)
     scatter!(
         scene, endpoints, marker = Makie.BezierUTriangle, color = cols,
-        rotations = dirs
+        rotation = dirs
     )
 end
 

docs/reference/plots/scatter.md

@@ -206,7 +206,7 @@ arrow_path = BezierPath([
 scatter(1:5,
     marker = arrow_path,
     markersize = range(20, 50, length = 5),
-    rotations = range(0, 2pi, length = 6)[1:end-1],
+    rotation = range(0, 2pi, length = 6)[1:end-1],
 )
 ```
 \end{examplefigure}
@@ -287,7 +287,7 @@ scatter(1:4, fill(0, 4), marker=Polygon(p_big, [p_small]), markersize=100, color
 
 ### Marker rotation
 
-Markers can be rotated using the `rotations` attribute, which also allows to pass a vector.
+Markers can be rotated using the `rotation` attribute, which also allows to pass a vector.
 
 \begin{examplefigure}{svg = true}
 ```julia
@@ -298,7 +298,7 @@ CairoMakie.activate!() # hide
 points = [Point2f(x, y) for y in 1:10 for x in 1:10]
 rotations = range(0, 2pi, length = length(points))
 
-scatter(points, rotations = rotations, markersize = 20, marker = '↑')
+scatter(points, rotation = rotations, markersize = 20, marker = '↑')
 ```
 \end{examplefigure}
 

src/basic_recipes/arrows.jl

@@ -177,7 +177,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
             lift(x-> last.(x), arrowplot, headstart),
             marker=marker_head,
             markersize = lift(as-> as === automatic ? theme(scene, :markersize)[] : as, arrowplot, arrowsize),
-            color = arrow_c, rotations = rotations, strokewidth = 0.0,
+            color = arrow_c, rotation = rotations, strokewidth = 0.0,
                  colormap=colormap, markerspace=arrowplot.markerspace, colorrange=arrowplot.colorrange,
             fxaa = fxaa_bool, inspectable = inspectable,
             transparency = transparency, visible = visible
@@ -211,7 +211,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
         marker_tail = lift((at, q) -> arrow_tail(3, at, q), arrowplot, arrowtail, quality)
         meshscatter!(
             arrowplot,
-            start, rotations = directions, markersize = msize,
+            start, rotation = directions, markersize = msize,
             marker = marker_tail,
             color = line_c, colormap = colormap, colorscale = colorscale, colorrange = arrowplot.colorrange,
             fxaa = fxaa_bool, ssao = ssao, shading = shading,
@@ -220,7 +220,7 @@ function plot!(arrowplot::Arrows{<: Tuple{AbstractVector{<: Point{N}}, V}}) wher
         )
         meshscatter!(
             arrowplot,
-            start, rotations = directions, markersize = markersize,
+            start, rotation = directions, markersize = markersize,
             marker = marker_head,
             color = arrow_c, colormap = colormap, colorscale = colorscale, colorrange = arrowplot.colorrange,
             fxaa = fxaa_bool, ssao = ssao, shading = shading,

src/basic_recipes/streamplot.jl

@@ -223,7 +223,7 @@ function plot!(p::StreamPlot)
     scatterfun(N)(
         p,
         lift(first, p, data);
-        markersize=arrow_size, rotations=rotations,
+        markersize=arrow_size, rotation=rotations,
         color=lift(x -> x[4], p, data),
         marker = lift((ah, q) -> arrow_head(N, ah, q), p, p.arrow_head, p.quality),
         colormap_args...,

src/conversions.jl

@@ -884,7 +884,6 @@ convert_attribute(x, key::Key) = x
 convert_attribute(color, ::key"color") = to_color(color)
 
 convert_attribute(colormap, ::key"colormap") = to_colormap(colormap)
-convert_attribute(rotation, ::key"rotation") = to_rotation(rotation)
 convert_attribute(font, ::key"font") = to_font(font)
 convert_attribute(align, ::key"align") = to_align(align)
 
@@ -919,11 +918,11 @@ function to_color(c::Tuple{<: Any,  <: Number})
     return RGBAf(Colors.color(col), alpha(col) * c[2])
 end
 
-convert_attribute(b::Billboard{Float32}, ::key"rotations") = to_rotation(b.rotation)
-convert_attribute(b::Billboard{Vector{Float32}}, ::key"rotations") = to_rotation.(b.rotation)
-convert_attribute(r::AbstractArray, ::key"rotations") = to_rotation.(r)
-convert_attribute(r::StaticVector, ::key"rotations") = to_rotation(r)
-convert_attribute(r, ::key"rotations") = to_rotation(r)
+convert_attribute(b::Billboard{Float32}, ::key"rotation") = to_rotation(b.rotation)
+convert_attribute(b::Billboard{Vector{Float32}}, ::key"rotation") = to_rotation.(b.rotation)
+convert_attribute(r::AbstractArray, ::key"rotation") = to_rotation.(r)
+convert_attribute(r::StaticVector, ::key"rotation") = to_rotation(r)
+convert_attribute(r, ::key"rotation") = to_rotation(r)
 
 convert_attribute(c, ::key"markersize", ::key"scatter") = to_2d_scale(c)
 convert_attribute(c, ::key"markersize", ::key"meshscatter") = to_3d_scale(c)

src/interaction/inspector.jl

@@ -455,7 +455,7 @@ function show_data(inspector::DataInspector, plot::MeshScatter, idx)
 
     if a.enable_indicators[]
         translation = apply_transform_and_model(plot, plot[1][][idx])
-        rotation = to_rotation(_to_rotation(plot.rotations[], idx))
+        rotation = to_rotation(_to_rotation(plot.rotation[], idx))
         scale = inv_f32_scale(plot, _to_scale(plot.markersize[], idx))
 
         bbox = Rect3d(convert_attribute(

src/layouting/boundingbox.jl

@@ -70,18 +70,18 @@ end
 # same as data_limits except using iterate_transformed
 function boundingbox(plot::MeshScatter)
     # TODO: avoid mesh generation here if possible
-    @get_attribute plot (marker, markersize, rotations)
+    @get_attribute plot (marker, markersize, rotation)
     marker_bb = Rect3d(marker)
     positions = iterate_transformed(plot)
     scales = markersize
     # fast path for constant markersize
-    if scales isa VecTypes{3} && rotations isa Quaternion
+    if scales isa VecTypes{3} && rotation isa Quaternion
         bb = Rect3d(positions)
-        marker_bb = rotations * (marker_bb * scales)
+        marker_bb = rotation * (marker_bb * scales)
         return Rect3d(minimum(bb) + minimum(marker_bb), widths(bb) + widths(marker_bb))
     else
         # TODO: optimize const scale, var rot and var scale, const rot
-        return limits_with_marker_transforms(positions, scales, rotations, marker_bb)
+        return limits_with_marker_transforms(positions, scales, rotation, marker_bb)
     end
 end