Skip to content

Commit 2652255

Browse files
asinghvi17claude
andcommitted
Add complete inverse transformation support to register_projected_positions!
- Add apply_inverse_transform, apply_inverse_transform_func, apply_inverse_float32convert, and apply_inverse_model kwargs - These enable correct projection from non-data spaces back to data space - Add early-exit optimization to skip redundant transform/inverse pairs when input_space === output_space - Add apply_inverse_model_to_positions helper function - Update docstring with new kwargs - Add tests for inverse transform_func 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 55aec6b commit 2652255

File tree

3 files changed

+170
-18
lines changed

3 files changed

+170
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Unreleased
44

5-
- Added `apply_inverse_float32convert` kwarg to `register_projected_positions!` to correctly apply inverse float32convert when projecting from non-data space to data space [#5485](https://github.com/MakieOrg/Makie.jl/pull/5485)
5+
- Added complete inverse transformation support to `register_projected_positions!` with `apply_inverse_transform`, `apply_inverse_transform_func`, `apply_inverse_float32convert`, and `apply_inverse_model` kwargs. These enable correct projection from non-data spaces back to data space. Includes early-exit optimization to skip redundant transform/inverse pairs when `input_space === output_space`. [#5485](https://github.com/MakieOrg/Makie.jl/pull/5485)
66
- Added loading spinner in WGLMakie that displays while the plot is being loaded [#5469](https://github.com/MakieOrg/Makie.jl/pull/5469)
77
- Moved decoration plots in `Axis3` to `ax.blockscene` so they no longer show up as user plots in the Axis3 [#5463](https://github.com/MakieOrg/Makie.jl/pull/5463)
88
- Fixed issue with `transformation` being applied multiple times when set by a user in a recipe that passes applicable attributes to child plots [#5464](https://github.com/MakieOrg/Makie.jl/pull/5464)

Makie/src/utilities/projection_utils.jl

Lines changed: 104 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@ to allow clip space clipping to happen elsewhere.)
1616
- `output_space = :pixel` sets the output space. Can be `:space` or `:markerspace` to refer to those plot attributes or any static space like `:pixel`.
1717
- `input_name = :positions` sets the source positions which will be projected.
1818
- `output_name = Symbol(output_space, :_, input_name)` sets the name of the projected positions.
19-
- `apply_transform = input_space === :space` controls whether transformations and float32convert are applied.
19+
- `apply_transform = input_space === :space` controls whether forward transformations and float32convert are applied.
2020
- `apply_transform_func = apply_transform` controls whether `transform_func` is applied.
2121
- `apply_float32convert = apply_transform` controls whether `float32convert` is applied.
2222
- `apply_model = apply_transform` controls whether the `model` matrix is applied.
2323
- `apply_clip_planes = false` controls whether points clipped by `clip_planes` are replaced by NaN. (Does not consider clip space clipping. Only applies if `is_data_space(input_space)`.)
24-
- `apply_inverse_float32convert = true` controls whether inverse `float32convert` is applied when projecting to data space.
24+
- `apply_inverse_transform = output_space === :space` controls whether inverse transformations are applied when projecting to data space.
25+
- `apply_inverse_transform_func = apply_inverse_transform` controls whether inverse `transform_func` is applied.
26+
- `apply_inverse_float32convert = apply_inverse_transform` controls whether inverse `float32convert` is applied.
27+
- `apply_inverse_model = apply_inverse_transform` controls whether inverse `model` matrix is applied.
2528
- `yflip = false` flips the `y` coordinate if set to true and `output_space = :pixel`
2629
2730
Related: [`register_position_transforms!`](@ref), [`register_positions_transformed!`](@ref),
@@ -42,23 +45,41 @@ function register_projected_positions!(
4245
input_name::Symbol = :positions,
4346
output_name::Symbol = Symbol(output_space, :_, input_name),
4447
yflip::Bool = false,
48+
# Forward transforms
4549
apply_transform::Bool = input_space === :space,
4650
apply_transform_func::Bool = apply_transform,
4751
apply_float32convert::Bool = apply_transform,
4852
apply_model::Bool = apply_transform,
4953
apply_clip_planes::Bool = false,
50-
apply_inverse_float32convert::Bool = true,
54+
# Inverse transforms
55+
apply_inverse_transform::Bool = output_space === :space,
56+
apply_inverse_transform_func::Bool = apply_inverse_transform,
57+
apply_inverse_float32convert::Bool = apply_inverse_transform,
58+
apply_inverse_model::Bool = apply_inverse_transform,
5159
) where {OT <: VecTypes}
5260

53-
# Handle transform function + f32c
54-
if apply_transform_func
61+
# Determine if spaces are data-like
62+
input_is_data_space = is_data_space(input_space) || input_space === :space
63+
output_is_data_space = is_data_space(output_space) || output_space === :space
64+
65+
# Early exit: skip transforms that will be immediately inverted
66+
# Inverse transforms only apply when: output_is_data_space && !input_is_data_space
67+
# So we can only skip when both forward AND inverse would ACTUALLY be applied
68+
inverse_would_apply = output_is_data_space && !input_is_data_space
69+
skip_transform_func = apply_transform_func && apply_inverse_transform_func && inverse_would_apply && input_space === output_space
70+
skip_float32convert = apply_float32convert && apply_inverse_float32convert && inverse_would_apply && input_space === output_space
71+
skip_model = apply_model && apply_inverse_model && inverse_would_apply && input_space === output_space
72+
73+
# Handle forward transform function
74+
if apply_transform_func && !skip_transform_func
5575
transformed_name = Symbol(input_name, :_transformed)
5676
register_positions_transformed!(plot_graph; input_name, output_name = transformed_name)
5777
else
5878
transformed_name = input_name
5979
end
6080

61-
if apply_float32convert && !is_data_space(output_space)
81+
# Handle forward float32convert (only when not targeting a static data space)
82+
if apply_float32convert && !skip_float32convert && !is_data_space(output_space)
6283
transformed_f32c_name = Symbol(transformed_name, :_f32c)
6384
register_positions_transformed_f32c!(plot_graph; input_name = transformed_name, output_name = transformed_f32c_name)
6485
else
@@ -67,35 +88,90 @@ function register_projected_positions!(
6788
transformed_f32c_name = transformed_name
6889
end
6990

70-
# Determine if we need post-projection inverse f32c
71-
# Only apply inverse f32c when projecting FROM non-data space TO data space
72-
input_is_data_space = is_data_space(input_space) || input_space === :space
73-
output_is_data_space = is_data_space(output_space) || output_space === :space
74-
needs_inv_f32c = apply_inverse_float32convert && output_is_data_space && !input_is_data_space
75-
projection_output = needs_inv_f32c ? Symbol(output_name, :_projected) : output_name
91+
# Determine which inverse transforms are needed
92+
# Inverse transforms only apply when projecting TO data space FROM non-data space
93+
needs_inv_f32c = apply_inverse_float32convert && !skip_float32convert && output_is_data_space && !input_is_data_space
94+
needs_inv_model = apply_inverse_model && !skip_model && output_is_data_space && !input_is_data_space
95+
needs_inv_tf = apply_inverse_transform_func && !skip_transform_func && output_is_data_space && !input_is_data_space
96+
needs_any_inverse = needs_inv_f32c || needs_inv_model || needs_inv_tf
97+
98+
# Use intermediate name for projection output if any inverse transforms will be applied
99+
projection_output = needs_any_inverse ? Symbol(output_name, :_projected) : output_name
76100

77101
if apply_model || (input_space !== output_space) || yflip
78102
register_positions_projected!(
79103
scene_graph, plot_graph, OT;
80104
input_space, output_space,
81105
input_name = transformed_f32c_name, output_name = projection_output,
82-
yflip, apply_model, apply_clip_planes
106+
yflip, apply_model = apply_model && !skip_model, apply_clip_planes
83107
)
84108
else
85109
ComputePipeline.alias!(plot_graph, transformed_f32c_name, projection_output)
86110
end
87111

88-
# Apply inverse f32c if needed
112+
# Apply inverse transforms after projection (in reverse order of forward application)
113+
current_output = projection_output
114+
115+
# 1. Inverse float32convert (innermost in forward pipeline)
89116
if needs_inv_f32c
117+
inv_f32c_name = Symbol(current_output, :_inv_f32c)
90118
if output_space === :space # dynamic
91-
map!(plot_graph, [projection_output, :f32c, :space], output_name) do pos, f32c, space
119+
map!(plot_graph, [current_output, :f32c, :space], inv_f32c_name) do pos, f32c, space
92120
return is_data_space(space) ? inv(f32c).(pos) : pos
93121
end
94122
else # static data space
95-
map!(plot_graph, [projection_output, :f32c], output_name) do pos, f32c
123+
map!(plot_graph, [current_output, :f32c], inv_f32c_name) do pos, f32c
96124
return inv(f32c).(pos)
97125
end
98126
end
127+
current_output = inv_f32c_name
128+
end
129+
130+
# 2. Inverse model
131+
if needs_inv_model
132+
inv_model_name = Symbol(current_output, :_inv_model)
133+
if output_space === :space # dynamic
134+
map!(plot_graph, [current_output, :model, :space], inv_model_name) do pos, model, space
135+
return is_data_space(space) ? apply_inverse_model_to_positions(model, pos) : pos
136+
end
137+
else # static data space
138+
map!(plot_graph, [current_output, :model], inv_model_name) do pos, model
139+
return apply_inverse_model_to_positions(model, pos)
140+
end
141+
end
142+
current_output = inv_model_name
143+
end
144+
145+
# 3. Inverse transform_func (outermost in forward pipeline)
146+
if needs_inv_tf
147+
inv_tf_name = Symbol(current_output, :_inv_tf)
148+
if output_space === :space # dynamic
149+
map!(plot_graph, [current_output, :transform_func, :space], inv_tf_name) do pos, tf, space
150+
inv_tf = inverse_transform(tf)
151+
if is_data_space(space) && !isnothing(inv_tf)
152+
# Use Makie. prefix to avoid shadowing by kwarg `apply_transform::Bool`
153+
return Makie.apply_transform(inv_tf, pos)
154+
else
155+
return pos
156+
end
157+
end
158+
else # static data space
159+
map!(plot_graph, [current_output, :transform_func], inv_tf_name) do pos, tf
160+
inv_tf = inverse_transform(tf)
161+
if isnothing(inv_tf)
162+
return pos
163+
else
164+
# Use Makie. prefix to avoid shadowing by kwarg `apply_transform::Bool`
165+
return Makie.apply_transform(inv_tf, pos)
166+
end
167+
end
168+
end
169+
current_output = inv_tf_name
170+
end
171+
172+
# Alias to final output name if different
173+
if current_output !== output_name
174+
ComputePipeline.alias!(plot_graph, current_output, output_name)
99175
end
100176

101177
return getindex(plot_graph, output_name)
@@ -169,7 +245,7 @@ function register_positions_projected!(
169245

170246
# apply projection
171247
# clip planes only apply from data/world space.
172-
if apply_clip_planes && is_data_space(input_space)
248+
if apply_clip_planes && (is_data_space(input_space) || input_space === :space)
173249
# easiest to transform them to the space of the projection input and
174250
# clip based on those points
175251
if apply_model
@@ -188,7 +264,9 @@ function register_positions_projected!(
188264
return _project(OT, matrix, pos, clip_planes, :data)
189265
end
190266
end
267+
191268
else
269+
# no clip planes, just project everything
192270
map!(plot_graph, [merged_matrix_name, input_name], output_name) do matrix, pos
193271
return _project(OT, matrix, pos)
194272
end
@@ -206,6 +284,15 @@ function register_f32c_matrix!(attr)
206284
return map!(attr, :f32c, :f32c_matrix)
207285
end
208286

287+
function apply_inverse_model_to_positions(model::Mat4, positions)
288+
inv_model = inv(model)
289+
return map(positions) do p
290+
p4d = to_ndim(Point4d, to_ndim(Point3d, p, 0), 1)
291+
p4d = inv_model * p4d
292+
return Point3f(p4d[Vec(1, 2, 3)] ./ p4d[4])
293+
end
294+
end
295+
209296
################################################################################
210297

211298
function register_markerspace_positions!(@nospecialize(plot::Plot), ::Type{OT} = Point3f; kwargs...) where {OT}

Makie/test/conversions/float32convert.jl

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,69 @@ using Makie: Mat4f, Vec2d, Vec3d, Point2d, Point3d, Point4d
380380
@test 499_000 < updated_positions[2][2] < 501_000
381381
end
382382

383+
@testset "register_projected_positions! inverse transform_func" begin
384+
# Test that projecting from pixel space to data space correctly
385+
# applies inverse transform_func (e.g., exp10 for log10 scale)
386+
387+
f, a, p = scatter(1:10, 10.0 .^ (1:10), axis = (yscale = log10,))
388+
Makie.update_state_before_display!(f)
389+
390+
# Verify we have a non-identity transform
391+
@test !Makie.is_identity_transform(p.transform_func[])
392+
393+
# First register pixel positions (they don't exist by default)
394+
Makie.register_projected_positions!(
395+
p;
396+
input_space = :data,
397+
output_space = :pixel,
398+
output_name = :test_pixel_positions,
399+
)
400+
Makie.update_state_before_display!(f)
401+
402+
pixel_positions = p.test_pixel_positions[]
403+
404+
# Create an input from pixel positions and project back to data space
405+
Makie.add_input!(p.attributes, :pixel_input, pixel_positions)
406+
Makie.register_projected_positions!(
407+
p;
408+
input_space = :pixel,
409+
output_space = :data,
410+
input_name = :pixel_input,
411+
output_name = :roundtrip_positions,
412+
apply_transform = false, # input is already in pixel space
413+
)
414+
Makie.update_state_before_display!(f)
415+
416+
roundtrip = p.roundtrip_positions[]
417+
original = p.positions[]
418+
419+
# Should roundtrip back to original data coordinates
420+
for i in eachindex(original)
421+
@test roundtrip[i][1] original[i][1] rtol = 0.01
422+
@test roundtrip[i][2] original[i][2] rtol = 0.01
423+
end
424+
end
425+
426+
@testset "register_projected_positions! early exit optimization" begin
427+
# Test that when input_space === output_space, transforms are skipped
428+
f, a, p = scatter(1:10, 10.0 .^ (1:10), axis = (yscale = log10,))
429+
Makie.update_state_before_display!(f)
430+
431+
# Register a projection from :data to :data - should skip transforms
432+
Makie.add_input!(p.attributes, :data_input, p.positions[])
433+
n_before = length(p.attributes.outputs)
434+
435+
Makie.register_projected_positions!(
436+
p;
437+
input_space = :data,
438+
output_space = :data,
439+
input_name = :data_input,
440+
output_name = :identity_output,
441+
)
442+
443+
# With early exit, fewer nodes should be created since transforms are skipped
444+
# The output should match the input exactly
445+
@test p.identity_output[] == p.data_input[]
446+
end
447+
383448
end

0 commit comments

Comments
 (0)