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

Allow GLMakie to display surfaces transformed by a 3d transform func #4243

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Bring back `poly` convert arguments for matrix with points as row [#4266](https://github.com/MakieOrg/Makie.jl/pull/4258).
- Fix gl_ClipDistance related segfault on WSL with GLMakie [#4270](https://github.com/MakieOrg/Makie.jl/pull/4270)
- Added option `label_position = :center` to place labels centered over each bar [#4274](https://github.com/MakieOrg/Makie.jl/pull/4274).
- Fix GLMakie `surface` implementation for 3D `transform_func` [#4243](https://github.com/MakieOrg/Makie.jl/pull/4243)

## [0.21.9] - 2024-08-27

Expand Down
23 changes: 12 additions & 11 deletions GLMakie/src/drawing_primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -820,33 +820,34 @@ function draw_atomic(screen::Screen, scene::Scene, plot::Surface)
if all(T -> T <: Union{AbstractMatrix, AbstractVector}, types)
t = Makie.transform_func_obs(plot)
mat = plot[3]
xypos = lift(plot, pop!(gl_attributes, :f32c), plot.model, t, plot[1], plot[2], space) do f32c, model, t, x, y, space
xyzpos = lift(plot, pop!(gl_attributes, :f32c), plot.model, t, plot[1], plot[2], mat, space) do f32c, model, t, x, y, z, space
# Only if transform doesn't do anything, we can stay linear in 1/2D
if Makie.is_identity_transform(t) && isnothing(f32c)
return (x, y)
return (x, y, z)
elseif Makie.is_translation_scale_matrix(model)
matrix = if x isa AbstractMatrix && y isa AbstractMatrix
Makie.f32_convert(f32c, apply_transform.((t,), Point.(x, y), space), space)
[Makie.f32_convert(f32c, apply_transform(t, Point3(x, y, z[ix, iy]), space), space) for (ix, x) in pairs(x), (iy, y) in pairs(y)]
Comment on lines 827 to +828
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this calling z[CartesianIndex(...), CartesianIndex(...)]?

Copy link
Collaborator

@ffreyer ffreyer Oct 18, 2024

Choose a reason for hiding this comment

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

Should probably also extend the refimg test to test

vec-vec vec-mat
mat-vec mat-mat

inputs

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, and will test that.

Copy link
Member Author

Choose a reason for hiding this comment

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

This could probably also be a loop operating on a pre-allocated array - that would be much clearer I think.

else
# If we do any transformation, we have to assume things aren't on the grid anymore
# so x + y need to become matrices.
[Makie.f32_convert(f32c, apply_transform(t, Point(x, y), space), space) for x in x, y in y]
[Makie.f32_convert(f32c, apply_transform(t, Point3(x, y, z[ix, iy]), space), space) for (ix, x) in pairs(x), (iy, y) in pairs(y)]
end
return (first.(matrix), last.(matrix))
return (getindex.(matrix, 1), getindex.(matrix, 2), getindex.(matrix, 3))
else
matrix = if x isa AbstractMatrix && y isa AbstractMatrix
Makie.f32_convert(f32c, apply_transform_and_model.((model,), (t,), Point.(x, y), space, Point2d), space)
[Makie.f32_convert(f32c, apply_transform_and_model(model, t, Point3.(x, y, z[ix, iy]), space, Point3d), space) for (ix, x) in pairs(x), (iy, y) in pairs(y)]
else
# If we do any transformation, we have to assume things aren't on the grid anymore
# so x + y need to become matrices.
[Makie.f32_convert(f32c, apply_transform_and_model(model, t, Point(x, y), space, Point2d), space) for x in x, y in y]
[Makie.f32_convert(f32c, apply_transform_and_model(model, t, Point3(x, y, z[ix, iy]), space, Point3d), space) for (ix, x) in pairs(x), (iy, y) in pairs(y)]
end
return (first.(matrix), last.(matrix))
return (getindex.(matrix, 1), getindex.(matrix, 2), getindex.(matrix, 3))
end
end
xpos = lift(first, plot, xypos)
ypos = lift(last, plot, xypos)
args = map((xpos, ypos, mat)) do arg
xpos = lift(Base.Fix2(getindex, 1), plot, xyzpos)
ypos = lift(Base.Fix2(getindex, 2), plot, xyzpos)
zpos = lift(Base.Fix2(getindex, 3), plot, xyzpos)
args = map((xpos, ypos, zpos)) do arg
Texture(lift(x-> convert(Array, el32convert(x)), plot, arg); minfilter=:linear)
end
if isnothing(img)
Expand Down
29 changes: 29 additions & 0 deletions ReferenceTests/src/tests/examples3d.jl
Original file line number Diff line number Diff line change
Expand Up @@ -734,3 +734,32 @@ end
ls2, pl = surface(f[1, 2], Makie.peaks(20); interpolate=false, axis=(; show_axis=false))
f
end

@reference_test "Surface with 3D transform_func" begin

SPHERICAL_TRANSFORM = Makie.PointTrans{3}() do geographic_point
# Longitude is directly translatable to a spherical coordinate
# θ (azimuth)
θ = deg2rad(geographic_point[1])
# The polar angle is 90 degrees minus the latitude
# ϕ (polar angle)
ϕ = deg2rad(90 - geographic_point[2])
# Finally, we define the base radius as 1,
# and all z values are offset by 1.
r = 1 + geographic_point[3]
# and we don't need to multiply by it.
return Point3(
r * sin(ϕ) * cos(θ),
r * sin(ϕ) * sin(θ),
r * cos(ϕ)
)
end

xs = -180:180
ys = -90:90
field = [exp(cosd(x)) + 3(y/90) for x in xs, y in ys] ./ 10
asinghvi17 marked this conversation as resolved.
Show resolved Hide resolved

f, a, p = surface(xs, ys, field; axis = (; type = LScene,), shading = NoShading)
p.transformation.transform_func[] = SPHERICAL_TRANSFORM
f
end
Loading