Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix errorbar and bracket interaction with transform functions #3012

Merged
merged 11 commits into from Jun 21, 2023
3 changes: 3 additions & 0 deletions NEWS.md
Expand Up @@ -2,6 +2,9 @@

## master

- Scale errorbar whiskers and bracket correctly with transformations [#3012](https://github.com/MakieOrg/Makie.jl/pull/3012)
- Update bracket when the screen is resized or transformations change [#3012](https://github.com/MakieOrg/Makie.jl/pull/3012)

## v0.19.6

- Fix broken AA for lines with strongly varying linewidth [#2953](https://github.com/MakieOrg/Makie.jl/pull/2953).
Expand Down
9 changes: 9 additions & 0 deletions ReferenceTests/src/tests/examples2d.jl
Expand Up @@ -337,6 +337,15 @@ end
fig
end

@reference_test "Errorbars log scale" begin
x = 1:5
y = sin.(x) .+ 5
fig = Figure()
errorbars(fig[1, 1], x, y, y .- 1, y .+ 1; linewidth = 3, whiskerwidth = 20, axis = (; yscale = log10, xscale = log10))
errorbars(fig[1, 2], y, x, y .- 1, y .+ 1; linewidth = 3, whiskerwidth = 20, direction = :x, axis = (; yscale = log10, xscale = log10))
fig
end

@reference_test "Rangebars x y low high" begin
vals = -1:0.1:1

Expand Down
16 changes: 10 additions & 6 deletions src/basic_recipes/bracket.jl
Expand Up @@ -54,16 +54,17 @@ function Makie.plot!(pl::Bracket)
return to === automatic ? Float32.(0.75 .* fs) : Float32.(to)
end

onany(pl, points, scene.camera.projectionview, pl.offset, pl.width, pl.orientation, realtextoffset,
pl.style) do points, pv, offset, width, orientation, textoff, style
onany(pl, points, scene.camera.projectionview, pl.model, transform_func(pl),
scene.px_area, pl.offset, pl.width, pl.orientation, realtextoffset,
pl.style) do points, _, _, _, _, offset, width, orientation, textoff, style

empty!(bp[])
empty!(textoffset_vec[])
empty!(textpoints[])

broadcast_foreach(points, offset, width, orientation, textoff, style) do (_p1, _p2), offset, width, orientation, textoff, style
p1 = scene_to_screen(_p1, scene)
p2 = scene_to_screen(_p2, scene)
p1 = plot_to_screen(pl, _p1)
p2 = plot_to_screen(pl, _p2)

v = p2 - p1
d1 = normalize(v)
Expand All @@ -83,6 +84,7 @@ function Makie.plot!(pl::Bracket)
end

notify(bp)
notify(textpoints)
end

notify(points)
Expand All @@ -105,10 +107,12 @@ function Makie.plot!(pl::Bracket)
return text isa AbstractString ? [text] : text
end

series!(pl, bp; space = :pixel, solid_color = pl.color, linewidth = pl.linewidth, linestyle = pl.linestyle)
# Avoid scale!() / translate!() / rotate!() to affect these
series!(pl, bp; space = :pixel, solid_color = pl.color, linewidth = pl.linewidth,
linestyle = pl.linestyle, transformation = Transformation())
text!(pl, textpoints, text = texts, space = :pixel, align = pl.align, offset = textoffset_vec,
fontsize = pl.fontsize, font = pl.font, rotation = autorotations, color = pl.textcolor,
justification = pl.justification)
justification = pl.justification, model = Mat4f(I))
Comment on lines +110 to +115
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently space = :pixel still includes model transformation (at least in GLMakie). Since plot_to_screen deals with them now, they'd be included twice. To prevent that I'm overwriting model/transformation. I'm using transformation for series because model doesn't get passed down.

pl
end

Expand Down
67 changes: 41 additions & 26 deletions src/basic_recipes/error_and_rangebars.jl
Expand Up @@ -187,19 +187,18 @@ function _plot_bars!(plot, linesegpairs, is_in_y_direction)

scene = parent_scene(plot)

whiskers = lift(plot, linesegpairs, scene.camera.projectionview,
scene.camera.pixel_space, whiskerwidth) do pairs, _, _, whiskerwidth
Comment on lines -190 to -191
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With camera.pixel_space this sometimes fails to update correctly when zooming or resizing. scene.px_area works fine.

whiskers = lift(plot, linesegpairs, scene.camera.projectionview, plot.model,
scene.px_area, transform_func(plot), whiskerwidth) do pairs, _, _, _, _, whiskerwidth

endpoints = [p for pair in pairs for p in pair]

screenendpoints = scene_to_screen(endpoints, scene)
screenendpoints = plot_to_screen(plot, endpoints)

screenendpoints_shifted_pairs = map(screenendpoints) do sep
(sep .+ f_if(is_in_y_direction[], reverse, Point(0, -whiskerwidth/2)),
sep .+ f_if(is_in_y_direction[], reverse, Point(0, whiskerwidth/2)))
end

screen_to_scene([p for pair in screenendpoints_shifted_pairs for p in pair], scene)
return [p for pair in screenendpoints_shifted_pairs for p in pair]
end
whiskercolors = Observable{RGBColors}()
map!(plot, whiskercolors, color) do color
Expand Down Expand Up @@ -229,37 +228,53 @@ function _plot_bars!(plot, linesegpairs, is_in_y_direction)
linesegments!(
plot, whiskers, color = whiskercolors, linewidth = whiskerlinewidths,
visible = visible, colormap = colormap, colorrange = colorrange,
inspectable = inspectable, transparency = transparency
inspectable = inspectable, transparency = transparency, space = :pixel,
model = Mat4f(I) # overwrite scale!() / translate!() / rotate!()
)
plot
end

function scene_to_screen(pts, scene)
p4 = to_ndim.(Vec4f, to_ndim.(Vec3f, pts, 0.0), 1.0)
p1m1 = Ref(scene.camera.projectionview[]) .* p4
projected = Ref(inv(scene.camera.pixel_space[])) .* p1m1
[Point2.(p[Vec(1, 2)]...) for p in projected]
function plot_to_screen(plot, points::AbstractVector)
cam = parent_scene(plot).camera
space = to_value(get(plot, :space, :data))
spvm = clip_to_space(cam, :pixel) * space_to_clip(cam, space) * transformationmatrix(plot)[]

return map(points) do p
transformed = apply_transform(transform_func(plot), p, space)
p4d = spvm * to_ndim(Point4f, to_ndim(Point3f, transformed, 0), 1)
return Point2f(p4d) / p4d[4]
end
end

function screen_to_scene(pts, scene)
p4 = to_ndim.(Vec4f, to_ndim.(Vec3f, pts, 0.0), 1.0)
p1m1 = Ref(scene.camera.pixel_space[]) .* p4
projected = Ref(inv(scene.camera.projectionview[])) .* p1m1
[Point2.(p[Vec(1, 2)]...) for p in projected]
function plot_to_screen(plot, p::VecTypes)
cam = parent_scene(plot).camera
space = to_value(get(plot, :space, :data))
spvm = clip_to_space(cam, :pixel) * space_to_clip(cam, space) * transformationmatrix(plot)[]
transformed = apply_transform(transform_func(plot), p, space)
p4d = spvm * to_ndim(Point4f, to_ndim(Point3f, transformed, 0), 1)
return Point2f(p4d) / p4d[4]
end

function scene_to_screen(p::T, scene) where T <: Point
p4 = to_ndim(Vec4f, to_ndim(Vec3f, p, 0.0), 1.0)
p1m1 = scene.camera.projectionview[] * p4
projected = inv(scene.camera.pixel_space[]) * p1m1
T(projected[Vec(1, 2)]...)
function screen_to_plot(plot, points::AbstractVector)
cam = parent_scene(plot).camera
space = to_value(get(plot, :space, :data))
mvps = inv(transformationmatrix(plot)[]) * clip_to_space(cam, space) * space_to_clip(cam, :pixel)
itf = inverse_transform(transform_func(plot))

return map(points) do p
pre_transform = mvps * to_ndim(Vec4f, to_ndim(Vec3f, p, 0.0), 1.0)
p3 = Point3f(pre_transform) / pre_transform[4]
return apply_transform(itf, p3, space)
end
end

function screen_to_scene(p::T, scene) where T <: Point
p4 = to_ndim(Vec4f, to_ndim(Vec3f, p, 0.0), 1.0)
p1m1 = scene.camera.pixel_space[] * p4
projected = inv(scene.camera.projectionview[]) * p1m1
T(projected[Vec(1, 2)]...)
function screen_to_plot(plot, p::VecTypes)
cam = parent_scene(plot).camera
space = to_value(get(plot, :space, :data))
mvps = inv(transformationmatrix(plot)[]) * clip_to_space(cam, space) * space_to_clip(cam, :pixel)
pre_transform = mvps * to_ndim(Vec4f, to_ndim(Vec3f, p, 0.0), 1.0)
p3 = Point3f(pre_transform) / pre_transform[4]
return apply_transform(itf, p3, space)
end

# ignore whiskers when determining data limits
Expand Down
2 changes: 1 addition & 1 deletion src/basic_recipes/text.jl
Expand Up @@ -57,7 +57,7 @@ function plot!(plot::Text)

onany(linesegs, positions, sc.camera.projectionview, sc.px_area,
transform_func_obs(sc), get(plot, :space, :data)) do segs, pos, _, _, transf, space
pos_transf = scene_to_screen(apply_transform(transf, pos, space), sc)
pos_transf = plot_to_screen(plot, pos)
linesegs_shifted[] = map(segs, lineindices[]) do seg, index
seg + attr_broadcast_getindex(pos_transf, index)
end
Expand Down