Skip to content

Commit

Permalink
Fixes for GeoMakie (#2645)
Browse files Browse the repository at this point in the history
* small fixes for GeoMakie

* interpolate module

* always get transformation from plot

* fix transformation

* Use the plot's transformation in more places in CairoMakie

* Make all CairoMakie primitives respect transform_func

* Use plot's space in transformation also

* Make data_lims actually transform

* Apply transform to heatmap, image, surface data limits

* Fix `update_cam!` for Scene

* Fix space not found

* Revert changes made to Axis

The issue still remains, but this causes failures in log scales and precompiles.  I've reimplemented the pipeline internally in GeoMakie, and will inspect why this fails at a later time.

* Actually remove all transformation

* Better comment on CairoMakie rasterization

* Use tuple instead of ref when mapping over image mesh in GLMakie

---------

Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com>
Co-authored-by: Anshul Singhvi <asinghvi17@simons-rock.edu>
  • Loading branch information
3 people authored Mar 15, 2023
1 parent b2b7c4e commit 7ff2df7
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 49 deletions.
10 changes: 6 additions & 4 deletions CairoMakie/src/infrastructure.jl
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ function cairo_draw(screen::Screen, scene::Scene)
end
Cairo.save(screen.context)

# This is a bit of a hack for now. When a plot is too large to save with
# a reasonable file size on a vector backend, the user can choose to
# rasterize it when plotting to vector backends, by using the `rasterize`
# keyword argument. This can be set to a Bool or an Int which describes
# When a plot is too large to save with a reasonable file size on a vector backend,
# the user can choose to rasterize it when plotting to vector backends, by using the
# `rasterize` keyword argument. This can be set to a Bool or an Int which describes
# the density of rasterization (in terms of a direct scaling factor.)
# TODO: In future, this can also be set to a Tuple{Module, Int} which describes
# the backend module which should be used to render the scene, and the pixel density
# at which it should be rendered.
if to_value(get(p, :rasterize, false)) != false && should_rasterize
draw_plot_as_image(pparent, screen, p, p[:rasterize][])
else # draw vector
Expand Down
43 changes: 23 additions & 20 deletions CairoMakie/src/primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
end

space = to_value(get(primitive, :space, :data))
projected_positions = project_position.(Ref(scene), Ref(space), positions, Ref(model))
projected_positions = project_position.(Ref(scene), (Makie.transform_func(primitive),), Ref(space), positions, Ref(model))

color = to_cairo_color(color, primitive)

Expand Down Expand Up @@ -224,7 +224,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scat
markerspace = to_value(get(primitive, :markerspace, :pixel))
space = to_value(get(primitive, :space, :data))

transfunc = scene.transformation.transform_func[]
transfunc = Makie.transform_func(primitive)

marker_conv = _marker_convert(marker)

Expand Down Expand Up @@ -427,7 +427,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Text

draw_glyph_collection(
scene, ctx, position, glyph_collection, remove_billboard(rotation),
model, space, markerspace, offset
model, space, markerspace, offset, primitive.transformation
)

nothing
Expand All @@ -436,21 +436,21 @@ end

function draw_glyph_collection(
scene, ctx, positions, glyph_collections::AbstractArray, rotation,
model::Mat, space, markerspace, offset
model::Mat, space, markerspace, offset, transformation
)

# TODO: why is the Ref around model necessary? doesn't broadcast_foreach handle staticarrays matrices?
broadcast_foreach(positions, glyph_collections, rotation, Ref(model), space,
markerspace, offset) do pos, glayout, ro, mo, sp, msp, off

draw_glyph_collection(scene, ctx, pos, glayout, ro, mo, sp, msp, off)
draw_glyph_collection(scene, ctx, pos, glayout, ro, mo, sp, msp, off, transformation)
end
end

_deref(x) = x
_deref(x::Ref) = x[]

function draw_glyph_collection(scene, ctx, position, glyph_collection, rotation, _model, space, markerspace, offsets)
function draw_glyph_collection(scene, ctx, position, glyph_collection, rotation, _model, space, markerspace, offsets, transformation)

glyphs = glyph_collection.glyphs
glyphoffsets = glyph_collection.origins
Expand All @@ -466,7 +466,7 @@ function draw_glyph_collection(scene, ctx, position, glyph_collection, rotation,
id = Mat4f(I)

glyph_pos = let
transform_func = scene.transformation.transform_func[]
transform_func = transformation.transform_func[]
p = Makie.apply_transform(transform_func, position, space)

Makie.clip_to_space(scene.camera, markerspace) *
Expand Down Expand Up @@ -603,7 +603,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
disable_fast_path = !fast_path
# Vector backends don't support FILTER_NEAREST for interp == false, so in that case we also need to draw rects
is_vector = is_vector_backend(ctx)
t = Makie.transform_func_obs(primitive)[]
t = Makie.transform_func(primitive)
identity_transform = (t === identity || t isa Tuple && all(x-> x === identity, t)) && (abs(model[1, 2]) < 1e-15)
regular_grid = xs isa AbstractRange && ys isa AbstractRange

Expand Down Expand Up @@ -650,7 +650,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
# find projected image corners
# this already takes care of flipping the image to correct cairo orientation
space = to_value(get(primitive, :space, :data))
xys = [project_position(scene, space, Point2f(x, y), model) for x in xs, y in ys]
xys = project_position.(scene, (Makie.transform_func(primitive),), space, [Point2f(x, y) for x in xs, y in ys], (model,))
colors = to_rgba_image(image, primitive)

# Note: xs and ys should have size ni+1, nj+1
Expand Down Expand Up @@ -734,10 +734,11 @@ function draw_mesh2D(scene, screen, @nospecialize(plot), @nospecialize(mesh))
lowclip, highclip, nan_color)

space = to_value(get(plot, :space, :data))::Symbol
return draw_mesh2D(scene, screen, cols, space, vs, fs, model)
transform_func = Makie.transform_func(plot)
return draw_mesh2D(scene, screen, cols, space, transform_func, vs, fs, model)
end

function draw_mesh2D(scene, screen, per_face_cols, space::Symbol,
function draw_mesh2D(scene, screen, per_face_cols, space::Symbol, transform_func,
vs::Vector{Point2f}, fs::Vector{GLTriangleFace}, model::Mat4f)

ctx = screen.context
Expand All @@ -746,7 +747,7 @@ function draw_mesh2D(scene, screen, per_face_cols, space::Symbol,

for (f, (c1, c2, c3)) in zip(fs, per_face_cols)
pattern = Cairo.CairoPatternMesh()
t1, t2, t3 = project_position.(scene, space, vs[f], (model,)) #triangle points
t1, t2, t3 = project_position.(scene, (transform_func,), space, vs[f], (model,)) #triangle points
Cairo.mesh_pattern_begin_patch(pattern)

Cairo.mesh_pattern_move_to(pattern, t1...)
Expand Down Expand Up @@ -802,16 +803,16 @@ function draw_mesh3D(scene, screen, attributes, mesh; pos = Vec4f(0), scale = 1f

model = attributes.model[]::Mat4f
space = to_value(get(attributes, :space, :data))::Symbol

func = Makie.transform_func(attributes)
draw_mesh3D(
scene, screen, space, meshpoints, meshfaces, meshnormals, per_face_col, pos, scale,
scene, screen, space, func, meshpoints, meshfaces, meshnormals, per_face_col, pos, scale,
model, shading::Bool, diffuse::Vec3f,
specular::Vec3f, shininess::Float32, faceculling::Int
)
end

function draw_mesh3D(
scene, screen, space, meshpoints, meshfaces, meshnormals, per_face_col, pos, scale,
scene, screen, space, transform_func, meshpoints, meshfaces, meshnormals, per_face_col, pos, scale,
model, shading, diffuse,
specular, shininess, faceculling
)
Expand All @@ -823,10 +824,10 @@ function draw_mesh3D(

# Mesh data
# transform to view/camera space
func = Makie.transform_func_obs(scene)[]
# pass func as argument to function, so that we get a function barrier
# and have `func` be fully typed inside closure
vs = broadcast(meshpoints, (func,)) do v, f

# pass transform_func as argument to function, so that we get a function barrier
# and have `transform_func` be fully typed inside closure
vs = broadcast(meshpoints, (transform_func,)) do v, f
# Should v get a nan2zero?
v = Makie.apply_transform(f, v, space)
p4d = to_ndim(Vec4f, scale .* to_ndim(Vec3f, v, 0f0), 1f0)
Expand Down Expand Up @@ -995,7 +996,9 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
color=color,
shading=primitive.shading, diffuse=primitive.diffuse,
specular=primitive.specular, shininess=primitive.shininess,
faceculling=get(primitive, :faceculling, -10)
faceculling=get(primitive, :faceculling, -10),
transformation=Makie.transformation(primitive)

)

if !(rotations isa Vector)
Expand Down
8 changes: 4 additions & 4 deletions GLMakie/src/drawing_primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ function draw_atomic(screen::Screen, scene::Scene,
return cached_robj!(screen, scene, x) do gl_attributes
glyphcollection = x[1]

transfunc = Makie.transform_func_obs(scene)
transfunc = Makie.transform_func_obs(x)
pos = gl_attributes[:position]
space = get(gl_attributes, :space, Observable(:data)) # needs to happen before connect_camera! call
markerspace = gl_attributes[:markerspace]
Expand Down Expand Up @@ -415,7 +415,7 @@ xy_convert(x, n) = Float32[LinRange(extrema(x)..., n + 1);]

function draw_atomic(screen::Screen, scene::Scene, x::Heatmap)
return cached_robj!(screen, scene, x) do gl_attributes
t = Makie.transform_func_obs(scene)
t = Makie.transform_func_obs(x)
mat = x[3]
space = get(gl_attributes, :space, :data) # needs to happen before connect_camera! call
xypos = map(t, x[1], x[2], space) do t, x, y, space
Expand Down Expand Up @@ -516,7 +516,7 @@ function mesh_inner(screen::Screen, mesh, transfunc, gl_attributes, space=:data)
end
mesh = map(mesh, transfunc, space) do mesh, func, space
if !Makie.is_identity_transform(func)
return update_positions(mesh, apply_transform.(Ref(func), mesh.position, space))
return update_positions(mesh, apply_transform.((func,), mesh.position, space))
end
return mesh
end
Expand Down Expand Up @@ -564,7 +564,7 @@ function draw_atomic(screen::Screen, scene::Scene, x::Surface)
types = map(v -> typeof(to_value(v)), x[1:2])

if all(T -> T <: Union{AbstractMatrix, AbstractVector}, types)
t = Makie.transform_func_obs(scene)
t = Makie.transform_func_obs(x)
mat = x[3]
xypos = map(t, x[1], x[2], space) do t, x, y, space
# Only if transform doesn't do anything, we can stay linear in 1/2D
Expand Down
2 changes: 1 addition & 1 deletion WGLMakie/src/particles.jl
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function create_shader(scene::Scene, plot::Makie.Text{<:Tuple{<:Union{<:Makie.Gl
glyphcollection = plot[1]
res = map(x->Vec2f(widths(x)), pixelarea(scene))
projview = scene.camera.projectionview
transfunc = Makie.transform_func_obs(scene)
transfunc = Makie.transform_func_obs(plot)
pos = plot.position
space = plot.space
markerspace = plot.markerspace
Expand Down
2 changes: 1 addition & 1 deletion src/camera/camera2d.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ update_cam!(scene::SceneLike, area) = update_cam!(scene, cameracontrols(scene),
Updates the camera for the given `scene` to cover the limits of the `Scene`.
Useful when using the `Observable` pipeline.
"""
update_cam!(scene::SceneLike) = update_cam!(scene, cameracontrols(scene), limits(scene)[])
update_cam!(scene::SceneLike) = update_cam!(scene, cameracontrols(scene), data_limits(scene))

function update_cam!(scene::Scene, cam::Camera2D, area3d::Rect)
area = Rect2f(area3d)
Expand Down
12 changes: 6 additions & 6 deletions src/layouting/data_limits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,14 @@ function iterate_transformed(plot)
points = point_iterator(plot)
t = transformation(plot)
model = model_transform(t)
# TODO: For some reason this was identity before and limit calculations in Axis with log scale are wrong if not, because they're already log transformed. What's the right behavior?
# trans_func = t.transform_func[]
trans_func = identity
iterate_transformed(points, model, trans_func)
# TODO: without this, axes with log scales error. Why?
trans_func = identity # transform_func(t)
# trans_func = identity
iterate_transformed(points, model, to_value(get(plot, :space, :data)), trans_func)
end

function iterate_transformed(points, model, trans_func)
(to_ndim(Point3f, project(model, apply_transform(trans_func, point)), 0f0) for point in points)
function iterate_transformed(points, model, space, trans_func)
(to_ndim(Point3f, project(model, apply_transform(trans_func, point, space)), 0f0) for point in points)
end

function update_boundingbox!(bb_ref, point)
Expand Down
10 changes: 4 additions & 6 deletions src/layouting/transformation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ function transform!(scene::Transformable, x::Tuple{Symbol, <: Number})
end

transformationmatrix(x) = transformation(x).model

transformation(x::Attributes) = x.transformation[]
transform_func(x) = transform_func_obs(x)[]
transform_func_obs(x) = transformation(x).transform_func

Expand All @@ -200,10 +200,10 @@ transform_func_obs(x) = transformation(x).transform_func
Apply the data transform func to the data if the space matches one
of the the transformation spaces (currently only :data is transformed)
"""
apply_transform(f, data, space) = space === :data ? apply_transform(f, data) : data
function apply_transform(f::Observable, data::Observable, space::Observable)
apply_transform(f, data, space) = space === :data ? apply_transform(f, data) : data
function apply_transform(f::Observable, data::Observable, space::Observable)
return lift((f, d, s)-> apply_transform(f, d, s), f, data, space)
end
end

"""
apply_transform(f, data)
Expand Down Expand Up @@ -383,5 +383,3 @@ end
# by heatmaps or images
zvalue2d(x)::Float32 = Makie.translation(x)[][3] + zvalue2d(x.parent)
zvalue2d(::Nothing)::Float32 = 0f0


4 changes: 2 additions & 2 deletions src/makielayout/blocks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ macro Block(name::Symbol, body::Expr = Expr(:block))

function Makie.default_attribute_values(::Type{$(name)}, scene::Union{Scene, Nothing})
sceneattrs = scene === nothing ? Attributes() : theme(scene)
curdeftheme = deepcopy(CURRENT_DEFAULT_THEME)
curdeftheme = deepcopy($(Makie).CURRENT_DEFAULT_THEME)
$(make_attr_dict_expr(attrs, :sceneattrs, :curdeftheme))
end

Expand Down Expand Up @@ -493,7 +493,7 @@ end

function Base.delete!(block::Block)
block.parent === nothing && return

# detach plots, cameras, transformations, px_area
empty!(block.blockscene)

Expand Down
7 changes: 4 additions & 3 deletions src/makielayout/blocks/axis.jl
Original file line number Diff line number Diff line change
Expand Up @@ -622,14 +622,15 @@ function reset_limits!(ax; xauto = true, yauto = true, zauto = true)
end
end

if ax isa Axis
ax.targetlimits[] = BBox(xlims..., ylims...)
tlims = if ax isa Axis
BBox(xlims..., ylims...)
elseif ax isa Axis3
ax.targetlimits[] = Rect3f(
Rect3f(
Vec3f(xlims[1], ylims[1], zlims[1]),
Vec3f(xlims[2] - xlims[1], ylims[2] - ylims[1], zlims[2] - zlims[1]),
)
end
ax.targetlimits[] = tlims
nothing
end

Expand Down
4 changes: 2 additions & 2 deletions src/makielayout/interactions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,11 @@ function process_interaction(r::RectangleZoom, event::KeysEvent, ax::Axis)
return Consume(true)
end

function positivize(r::Rect2f)
function positivize(r::Rect2)
negwidths = r.widths .< 0
newori = ifelse.(negwidths, r.origin .+ r.widths, r.origin)
newwidths = ifelse.(negwidths, -r.widths, r.widths)
return Rect2f(newori, newwidths)
return Rect2(newori, newwidths)
end

function process_interaction(l::LimitReset, event::MouseEvent, ax::Axis)
Expand Down

0 comments on commit 7ff2df7

Please sign in to comment.