From 132ed3f83794f6554609014f7bbf84c36f374020 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Wed, 9 Oct 2024 12:46:38 -0700 Subject: [PATCH 1/5] Make `VideoStream` aware of `px_per_unit` --- src/ffmpeg-util.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index b7e7b1bb55c..c090f3a3515 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -260,7 +260,7 @@ function VideoStream(fig::FigureLike; get!(config, :visible, visible) get!(config, :start_renderloop, false) screen = getscreen(backend, scene, config, GLNative) - _xdim, _ydim = size(screen) + _xdim, _ydim = ceil.(Int, size(screen) .* screen.config.px_per_unit) xdim = iseven(_xdim) ? _xdim : _xdim + 1 ydim = iseven(_ydim) ? _ydim : _ydim + 1 buffer = Matrix{RGB{N0f8}}(undef, xdim, ydim) From fd6e3abff185f07b07a77bab518220f0cefa89e8 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Thu, 10 Oct 2024 14:46:56 -0700 Subject: [PATCH 2/5] Make this robust against lack of keys (either config or px_per_unit) --- src/ffmpeg-util.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index c090f3a3515..7654c7a46a9 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -260,7 +260,13 @@ function VideoStream(fig::FigureLike; get!(config, :visible, visible) get!(config, :start_renderloop, false) screen = getscreen(backend, scene, config, GLNative) - _xdim, _ydim = ceil.(Int, size(screen) .* screen.config.px_per_unit) + # We extract `px_per_unit` from the screen in order to get the ground truth. + px_per_unit = if hasproperty(screen, :config) && hasproperty(screen.config, :px_per_unit) + screen.config.px_per_unit + else # backend has no screen config, or config has no px_per_unit, so assume 1 px per dip + 1 + end + _xdim, _ydim = ceil.(Int, size(screen) .* px_per_unit) xdim = iseven(_xdim) ? _xdim : _xdim + 1 ydim = iseven(_ydim) ? _ydim : _ydim + 1 buffer = Matrix{RGB{N0f8}}(undef, xdim, ydim) From 0c346f6f20bbf2f0123caca58d630374f504fc29 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Thu, 10 Oct 2024 14:48:12 -0700 Subject: [PATCH 3/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef871fb1f7..cf3d5a31496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] - Fix relocatability of GLMakie [#4461](https://github.com/MakieOrg/Makie.jl/pull/4461). +- Fix `px_per_unit` not being recognized in `record` and `VideoStream`, leading to errors [#4466](https://github.com/MakieOrg/Makie.jl/pull/4466). ## [0.21.13] - 2024-10-07 From bf2831a524935da40eb7debfe02c4c4add52dfa1 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Thu, 10 Oct 2024 16:56:14 -0700 Subject: [PATCH 4/5] Add a test to record.jl --- test/record.jl | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/test/record.jl b/test/record.jl index 9e2310a28ed..af786b0a0f7 100644 --- a/test/record.jl +++ b/test/record.jl @@ -2,16 +2,18 @@ using Logging module VideoBackend using Makie + @kwdef struct ScreenConfig + px_per_unit = 1 + end struct Screen <: MakieScreen size::Tuple{Int, Int} - end - struct ScreenConfig + config::ScreenConfig end Base.size(screen::Screen) = screen.size - Screen(scene::Scene, config::ScreenConfig, ::Makie.ImageStorageFormat) = Screen(size(scene)) + Screen(scene::Scene, config::ScreenConfig, ::Makie.ImageStorageFormat) = Screen(size(scene), config) Makie.backend_showable(::Type{Screen}, ::MIME"text/html") = true Makie.backend_showable(::Type{Screen}, ::MIME"image/png") = true - Makie.colorbuffer(screen::Screen) = zeros(RGBf, reverse(screen.size)...) + Makie.colorbuffer(screen::Screen) = zeros(RGBf, ceil.(Int, reverse(screen.size) .* screen.config.px_per_unit)...) Base.display(::Screen, ::Scene; kw...) = nothing end @@ -39,6 +41,20 @@ mktempdir() do tempdir end end + @testset "px_per_unit" begin + for fmt in ("mp4", "gif"), px_per_unit in (1, 2) + dst = joinpath(tempdir, "out_$px_per_unit.$fmt") + @test begin + record(fig, dst, 1:n; px_per_unit) do i + lines!(ax, sin.(i .* x)) + return nothing + end + true + end + end + end + + # test that the proper warnings are thrown @testset "Warnings" begin function run_record(dst; kwargs...) From 651ff70781d652593f86b6a0de7d1feb1f8aa1c8 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Fri, 11 Oct 2024 08:58:59 -0700 Subject: [PATCH 5/5] Update record.jl --- test/record.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/record.jl b/test/record.jl index af786b0a0f7..3464e0499ca 100644 --- a/test/record.jl +++ b/test/record.jl @@ -2,7 +2,7 @@ using Logging module VideoBackend using Makie - @kwdef struct ScreenConfig + Base.@kwdef struct ScreenConfig px_per_unit = 1 end struct Screen <: MakieScreen