From 6848f0120e2295df63b7dd58a8dafd079cd1f43e Mon Sep 17 00:00:00 2001 From: ffreyer Date: Mon, 19 Feb 2024 16:56:08 +0100 Subject: [PATCH 1/4] fix #3639 --- GLMakie/src/drawing_primitives.jl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/GLMakie/src/drawing_primitives.jl b/GLMakie/src/drawing_primitives.jl index 704e456b927..918f1b5ad1e 100644 --- a/GLMakie/src/drawing_primitives.jl +++ b/GLMakie/src/drawing_primitives.jl @@ -181,9 +181,14 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] end # resolution in real hardware pixels, not scaled pixels/units get!(gl_attributes, :resolution) do - get!(cam.calculated_values, :resolution) do - return lift(*, plot, gl_attributes[:px_per_unit], cam.resolution) - end + # Note: + # robj cleanup deletes all uniforms so to avoid deleting the cached + # resolution we need to create a copy here. + # TODO closing the screen should delete the cached entry as px_per_unit + # could be different in the next screen + lift(identity, get!(cam.calculated_values, :resolution) do + return lift(*, gl_attributes[:px_per_unit], cam.resolution) + end) end delete!(gl_attributes, :space) From 790144efe008bdb3314380dda34d902e06ebb085 Mon Sep 17 00:00:00 2001 From: ffreyer Date: Mon, 19 Feb 2024 17:40:40 +0100 Subject: [PATCH 2/4] re-enable caching --- GLMakie/src/drawing_primitives.jl | 36 ++++++++++++++----------------- src/camera/camera.jl | 27 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/GLMakie/src/drawing_primitives.jl b/GLMakie/src/drawing_primitives.jl index 918f1b5ad1e..3d109cefe42 100644 --- a/GLMakie/src/drawing_primitives.jl +++ b/GLMakie/src/drawing_primitives.jl @@ -142,12 +142,21 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] # Overwrite these, user defined attributes shouldn't use those! gl_attributes[key] = lift(identity, plot, getfield(cam, key)) end + get!(gl_attributes, :view) do - # get!(cam.calculated_values, Symbol("view_$(space[])")) do - return lift(plot, cam.view, space) do view, space - return is_data_space(space) ? view : Mat4f(I) - end - # end + return lift(cam.view, space) do _, space + return Makie.get_cached_matrix(cam, :view, space) + end + end + get!(gl_attributes, :projection) do + return lift(cam.projection, cam.pixel_space, space) do _, _, space + return Makie.get_cached_matrix(cam, :projection, space) + end + end + get!(gl_attributes, :projectionview) do + return lift(cam.projectionview, cam.pixel_space, space) do _, _, space + return Makie.get_cached_matrix(cam, :projectionview, space) + end end # for lighting @@ -165,20 +174,7 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] return transpose(inv(v[i, i] * m[i, i])) end end - get!(gl_attributes, :projection) do - # return get!(cam.calculated_values, Symbol("projection_$(space[])")) do - return lift(plot, cam.projection, cam.pixel_space, space) do _, _, space - return Makie.space_to_clip(cam, space, false) - end - # end - end - get!(gl_attributes, :projectionview) do - # get!(cam.calculated_values, Symbol("projectionview_$(space[])")) do - return lift(plot, cam.projectionview, cam.pixel_space, space) do _, _, space - Makie.space_to_clip(cam, space, true) - end - # end - end + # resolution in real hardware pixels, not scaled pixels/units get!(gl_attributes, :resolution) do # Note: @@ -186,7 +182,7 @@ function connect_camera!(plot, gl_attributes, cam, space = gl_attributes[:space] # resolution we need to create a copy here. # TODO closing the screen should delete the cached entry as px_per_unit # could be different in the next screen - lift(identity, get!(cam.calculated_values, :resolution) do + return lift(identity, get!(cam.calculated_values, :px_resolution) do return lift(*, gl_attributes[:px_per_unit], cam.resolution) end) end diff --git a/src/camera/camera.jl b/src/camera/camera.jl index 2d75f3dc9ca..1188d8d6e01 100644 --- a/src/camera/camera.jl +++ b/src/camera/camera.jl @@ -123,3 +123,30 @@ function is_mouseinside(scene::Scene) # is_mouseinside(child) && return false # end end + +function get_cached_obs(cam::Camera, name::Symbol, space::Symbol) + fullname = Symbol("$(name)_$(space)") + obs = get!(cam.calculated_values, fullname) do + # These run at max priority so they update before default priority + # listeners to projectionview etc are called. + if name == :view + return lift(cam.view, priority = typemax(Int)) do view + return is_data_space(space) ? view : Mat4f(I) + end + elseif name == :projection + return lift(cam.projection, cam.pixel_space, priority = typemax(Int)) do _, _ + return Makie.space_to_clip(cam, space, false) + end + elseif name == :projectionview + return lift(cam.projectionview, cam.pixel_space, priority = typemax(Int)) do _, _ + return Makie.space_to_clip(cam, space, true) + end + else + error("Cannot create a cached Observable for unrecognized camera matrix :$(name).") + end + end + return obs +end +@inline function get_cached_matrix(cam::Camera, name::Symbol, space::Symbol) + return get_cached_obs(cam, name, space)[] +end \ No newline at end of file From 78b9eb9f68e353051e9503509c46a126aef9fdb6 Mon Sep 17 00:00:00 2001 From: ffreyer Date: Mon, 19 Feb 2024 18:29:11 +0100 Subject: [PATCH 3/4] add test --- GLMakie/test/unit_tests.jl | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl index 409247bc075..52c184eb747 100644 --- a/GLMakie/test/unit_tests.jl +++ b/GLMakie/test/unit_tests.jl @@ -443,3 +443,25 @@ end im[3][] = zeros(RGBf, 25, 15) # larger size GLMakie.closeall() end + +@testset "Camera caching" begin + f=Figure(size=(200,200)) + screen = display(f, visible = false) + ax=Axis(f[1,1]) + lines!(ax,sin.(0.0:0.1:2pi)) + text!(ax,10.0,0.0,text="sine wave") + expected_keys = [ + :projection_data, :view_data, :projectionview_pixel, :projectionview_data, + :px_resolution, :projection_pixel, :view_pixel + ] + for key in expected_keys + @test haskey(ax.scene.camera.calculated_values, key) + @test !isempty(ax.scene.camera.calculated_values[key].inputs) + end + empty!(ax) + for key in expected_keys + @test haskey(ax.scene.camera.calculated_values, key) + @test !isempty(ax.scene.camera.calculated_values[key].inputs) + end + GLMakie.closeall() +end \ No newline at end of file From 1e1a9b4eeecea02049fe69eae884442fe92e4df7 Mon Sep 17 00:00:00 2001 From: ffreyer Date: Mon, 19 Feb 2024 18:33:38 +0100 Subject: [PATCH 4/4] document change --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 110e3389099..9e906d8993b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fixed bugs with format strings and add new features by switching to Format.jl [#3633](https://github.com/MakieOrg/Makie.jl/pull/3633). - Fixed an issue where CairoMakie would unnecessarily rasterize polygons [#3605](https://github.com/MakieOrg/Makie.jl/pull/3605). - Added `PointBased` conversion trait to `scatterlines` recipe [#3603](https://github.com/MakieOrg/Makie.jl/pull/3603). +- Fixed a bug with line/text/scatter resolutions getting disconnected by `empty!()` in GLMakie and re-enabled camera caching. [#3640](https://github.com/MakieOrg/Makie.jl/pull/3640) ## [0.20.7] - 2024-02-04