Fix register_positions_projected! not updating when float32convert changes

[asinghvi17]

Summary

• Fixes register_positions_projected! not updating output positions when float32convert changes
• When projecting from non-data space (:relative, :pixel) to data space, applies inverse f32c to get actual data coordinates
• Adds :f32c as a dependency so outputs update when float32convert changes

Problem

When using register_positions_projected! with input_space = :relative and output_space = :space, the output positions were incorrect after float32convert changed (e.g., after calling limits! to zoom into a small region).

The root cause is that camera matrices operate in f32c-scaled space (the projection is set based on f32_convert(scene.float32convert, limits)). When transforming from a non-data space to data space, the output needs the inverse of float32convert applied, but this wasn't happening.

Limitation

This fix does not yet account for transform_func when projecting from non-data space to data space.

Test plan

• Added test case that verifies positions update correctly when float32convert changes
• Existing tests pass

🤖 Generated with Claude Code

[ffreyer]

This should not happen in register_positions_projected!(). It should be part of register_projected_positions!(). The former just does matrix related transformations, i.e. projections (and clip planes because those happen in those coordinate systems), the latter does that and transform func and float32convert. (I know the naming sucks, I couldn't think of anything good when I introduced them)

We should also not do the inverse float32convert and transform_func solely based on space === :data because input data, transformed data (transform_func) and f32c transformed data all use space = :data. We should have something else to clear up that ambiguity, which is why the latter function has apply_transform, apply_transform_func etc. Since we could technically have e.g. f32c transformed data -> input data we should probably just add options like apply_inverse_transform and so on here. (Otherwise how do we decide what's transformed and what's not?)

[MakieBot]

Benchmark Results

SHA: 8d84cce5252acbcefd1a3a89c83fd6eedf0d49a2

Warning

These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking.

[asinghvi17]

That sounds reasonable but then we should probably point users to the function you just mentioned rather than here?

Otherwise how do we decide what's transformed and what's not?

I know it's a broken record at this point but space = :transformed... :P

[ffreyer]

That sounds reasonable but then we should probably point users to the function you just mentioned rather than here?

Where are we doing that? Both the bits I added to the online docs don't:
https://docs.makie.org/stable/explanations/recipes#recipe_projections
https://docs.makie.org/stable/explanations/conversion_pipeline#pipeline_recipe_projections

I know it's a broken record at this point but space = :transformed... :P

And my broken record answer is that transformations can apply to any space and we'd need a bunch of variants of transformed. Why wouldn't translate!() be ok in pixel space? Or some transform function? And multiplicatively to that we'd have the options of including or not including transformations. This also matters for f32convert because the order of operations is f32c -> model -> space-related projections. As I see it we'd need all of these if we go down that road:

model_data
model_pixel
model_clip
model_relative

f32c_model_data
f32c_data

transformed_f32c_model_data
transformed_model_data
transformed_model_pixel
transformed_model_clip
transformed_model_relative
transformed_f32c_data
transformed_data
transformed_pixel
transformed_clip
transformed_relative

I wanted to avoid that by just keeping transformations and projections separate.

---

This is still changing code in register_positions_projected, which might be why tests fail?

I think this pr should just do inverse model and inverse transform function too. And keep the style of the forward transformations. I.e. have apply_inverse_transform = output_space === :space; apply_inverse_transform_func = apply_inverse_transform etc in the function header.

I think it's fine to just assume the inverse transform function exists when apply_inverse_transform_func is true. (And it shouldn't really matter if it's expensive to produce - just cache it in the compute graph. Inverse model and inverse f32c would probably also make sense to cache.)

We should probably also have earlier exit conditions to avoid stuff like inverse_transform(transform(data))

[asinghvi17]

Hm it's possible I looked at some recipe implementation or so and got this from there, or had an earlier concept that I took some code from.

I see your point about the combinatorial explosion you get since we now allow transform_func on any space, which would become a problem...

Will get claude to add the other two options as well.

[asinghvi17]

Bumping on this, it should have everything requested now (I think)

[ffreyer]

I'm still unsure what to do with the default for apply_inverse_transform. To make this a completely non-breaking change (with my changes) it would need to be false and apply_inverse_float32convert = is_data_space(output_space).
With that output_space = :space would trigger a forward transform_func and a forward model application without a float32convert, rather than skipping everything. I think the old version is more useful here, though technically also a bit risky/wrong since projections use Float32 matrices and don't know whether float32converts have been applied or not, i.e. they might pick the wrong model matrix.