HTML video size annotation (#4563)

* Annotate inline video with logical video size for correct scaling

* add test

* changelog entry

* use `size(scene)`

* rename `root_scene` to `scene` and add unit test

CHANGELOG.md

@@ -9,6 +9,7 @@
 - Expand PlotList plots to expose their child plots to the legend interface, allowing `axislegend`show plots within PlotSpecs as individual entries. [#4546](https://github.com/MakieOrg/Makie.jl/pull/4546)
 - Implement S.Colorbar(plotspec) [#4520](https://github.com/MakieOrg/Makie.jl/pull/4520).
 - Fixed a hang when `Record` was created inside a closure passed to `IOCapture.capture` [#4562](https://github.com/MakieOrg/Makie.jl/pull/4562).
+- Added logical size annotation to `text/html` inline videos so that sizes are appropriate independent of the current `px_per_unit` value [#4563](https://github.com/MakieOrg/Makie.jl/pull/4563).
 
 ## [0.21.15] - 2024-10-25
 

CairoMakie/test/runtests.jl

@@ -122,7 +122,9 @@ end
 @testset "VideoStream & screen options" begin
     N = 3
     points = Observable(Point2f[])
-    f, ax, pl = scatter(points, axis=(type=Axis, aspect=DataAspect(), limits=(0.4, N + 0.6, 0.4, N + 0.6),), figure=(size=(600, 800),))
+    width = 600
+    height = 800
+    f, ax, pl = scatter(points, axis=(type=Axis, aspect=DataAspect(), limits=(0.4, N + 0.6, 0.4, N + 0.6),), figure=(size=(width, height),))
 
     vio = Makie.VideoStream(f; format="mp4", px_per_unit=2.0, backend=CairoMakie)
     tmp_path = vio.path
@@ -132,6 +134,11 @@ end
     @test vio.screen.device_scaling_factor == 2.0
 
     Makie.recordframe!(vio)
+
+    html = repr(MIME"text/html"(), vio)
+    @test occursin("width=\"$width\"", html)
+    @test occursin("height=\"$height\"", html)
+
     save("test.mp4", vio)
     save("test_2.mkv", vio)
     save("test_3.mp4", vio)

GLMakie/src/display.jl

@@ -2,13 +2,13 @@ function Base.display(screen::Screen, scene::Scene; connect=true)
     # So, the GLFW window events are not guarantee to fire
     # when we close a window, so we ensure this here!
     if !Makie.is_displayed(screen, scene)
-        if !isnothing(screen.root_scene)
-            delete!(screen, screen.root_scene)
-            screen.root_scene = nothing
+        if !isnothing(screen.scene)
+            delete!(screen, screen.scene)
+            screen.scene = nothing
         end
         display_scene!(screen, scene)
     else
-        @assert screen.root_scene === scene "internal error. Scene already displayed by screen but not as root scene"
+        @assert screen.scene === scene "internal error. Scene already displayed by screen but not as root scene"
     end
     pollevents(screen, Makie.BackendTick)
     return screen

GLMakie/src/picking.jl

@@ -12,7 +12,7 @@ function pick_native(screen::Screen, rect::Rect2i)
     glReadBuffer(GL_COLOR_ATTACHMENT1)
     rx, ry = minimum(rect)
     rw, rh = widths(rect)
-    w, h = size(screen.root_scene)
+    w, h = size(screen.scene)
     ppu = screen.px_per_unit[]
     if rx >= 0 && ry >= 0 && rx + rw <= w && ry + rh <= h
         rx, ry, rw, rh = round.(Int, ppu .* (rx, ry, rw, rh))
@@ -32,7 +32,7 @@ function pick_native(screen::Screen, xy::Vec{2, Float64})
     glBindFramebuffer(GL_FRAMEBUFFER, fb.id[1])
     glReadBuffer(GL_COLOR_ATTACHMENT1)
     x, y = floor.(Int, xy)
-    w, h = size(screen.root_scene)
+    w, h = size(screen.scene)
     ppu = screen.px_per_unit[]
     if x > 0 && y > 0 && x <= w && y <= h
         x, y = round.(Int, ppu .* (x, y))
@@ -67,7 +67,7 @@ end
 # Skips one set of allocations
 function Makie.pick_closest(scene::Scene, screen::Screen, xy, range)
     isopen(screen) || return (nothing, 0)
-    w, h = size(screen.root_scene) # unitless dimensions
+    w, h = size(screen.scene) # unitless dimensions
     ((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h)) || return (nothing, 0)
 
     fb = screen.framebuffer
@@ -106,7 +106,7 @@ end
 # Skips some allocations
 function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range)
     isopen(screen) || return Tuple{AbstractPlot, Int}[]
-    w, h = size(screen.root_scene) # unitless dimensions
+    w, h = size(screen.scene) # unitless dimensions
     if !((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h))
         return Tuple{AbstractPlot, Int}[]
     end

GLMakie/src/rendering.jl

@@ -1,8 +1,8 @@
 function setup!(screen::Screen)
     glEnable(GL_SCISSOR_TEST)
-    if isopen(screen) && !isnothing(screen.root_scene)
+    if isopen(screen) && !isnothing(screen.scene)
         ppu = screen.px_per_unit[]
-        glScissor(0, 0, round.(Int, size(screen.root_scene) .* ppu)...)
+        glScissor(0, 0, round.(Int, size(screen.scene) .* ppu)...)
         glClearColor(1, 1, 1, 1)
         glClear(GL_COLOR_BUFFER_BIT)
         for (id, scene) in screen.screens
@@ -43,9 +43,9 @@ function render_frame(screen::Screen; resize_buffers=true)
     # render order here may introduce artifacts because of that.
 
     fb = screen.framebuffer
-    if resize_buffers && !isnothing(screen.root_scene)
+    if resize_buffers && !isnothing(screen.scene)
         ppu = screen.px_per_unit[]
-        resize!(fb, round.(Int, ppu .* size(screen.root_scene))...)
+        resize!(fb, round.(Int, ppu .* size(screen.scene))...)
     end
 
     # prepare stencil (for sub-scenes)

GLMakie/src/screen.jl

@@ -180,7 +180,7 @@ mutable struct Screen{GLWindow} <: MakieScreen
     window_open::Observable{Bool}
     scalefactor::Observable{Float32}
 
-    root_scene::Union{Scene, Nothing}
+    scene::Union{Scene, Nothing}
     reuse::Bool
     close_after_renderloop::Bool
     # To trigger rerenders that aren't related to an existing renderobject.
@@ -400,8 +400,8 @@ function apply_config!(screen::Screen, config::ScreenConfig; start_renderloop::B
     else
         stop_renderloop!(screen)
     end
-    if !isnothing(screen.root_scene)
-        resize!(screen, size(screen.root_scene)...)
+    if !isnothing(screen.scene)
+        resize!(screen, size(screen.scene)...)
     end
     set_screen_visibility!(screen, config.visible)
     return screen
@@ -442,7 +442,7 @@ function display_scene!(screen::Screen, scene::Scene)
     insertplots!(screen, scene)
     Makie.push_screen!(scene, screen)
     connect_screen(scene, screen)
-    screen.root_scene = scene
+    screen.scene = scene
     return
 end
 
@@ -612,10 +612,10 @@ function Base.empty!(screen::Screen)
         delete!(screen, Makie.rootparent(plot), plot)
     end
 
-    if !isnothing(screen.root_scene)
-        Makie.disconnect_screen(screen.root_scene, screen)
-        delete!(screen, screen.root_scene)
-        screen.root_scene = nothing
+    if !isnothing(screen.scene)
+        Makie.disconnect_screen(screen.scene, screen)
+        delete!(screen, screen.scene)
+        screen.scene = nothing
     end
 
     @assert isempty(screen.renderlist)
@@ -875,8 +875,8 @@ end
 scalechangecb(screen) = (window, xscale, yscale) -> scalechangecb(screen, window, xscale, yscale)
 
 function scalechangeobs(screen, _)
-    if !isnothing(screen.root_scene)
-        resize!(screen, size(screen.root_scene)...)
+    if !isnothing(screen.scene)
+        resize!(screen, size(screen.scene)...)
     end
     return nothing
 end

GLMakie/test/unit_tests.jl

@@ -288,7 +288,7 @@ end
         @test isempty(screen.px_per_unit.listeners)
         @test isempty(screen.scalefactor.listeners)
 
-        @test screen.root_scene === nothing
+        @test screen.scene === nothing
         @test screen.rendertask === nothing
         @test (Base.summarysize(screen) / 10^6) < 1.4
     end
@@ -438,6 +438,22 @@ end
     GLMakie.closeall()
 end
 
+@testset "html video size annotation" begin
+    width = 600
+    height = 800
+    f = Figure(; size = (width, height))
+
+    vio = Makie.VideoStream(f; format="mp4", px_per_unit=2.0, backend=GLMakie)
+
+    @test size(vio.screen) == size(f.scene) .* 2
+
+    Makie.recordframe!(vio)
+
+    html = repr(MIME"text/html"(), vio)
+    @test occursin("width=\"$width\"", html)
+    @test occursin("height=\"$height\"", html)
+end
+
 @testset "image size changes" begin
     s = Scene()
     im = image!(s, 0..10, 0..10, zeros(RGBf, 10, 20))

src/recording.jl

@@ -179,13 +179,18 @@ function Record(func, figlike, iter; kw_args...)
 end
 
 function Base.show(io::IO, ::MIME"text/html", vs::VideoStream)
+    scene = vs.screen.scene
+    if !(scene isa Scene)
+        error("Expected Screen to hold a reference to a Scene but got $(repr(scene))")
+    end
+    w, h = size(scene)
     mktempdir() do dir
         path = save(joinpath(dir, "video.mp4"), vs)
         # <video> only supports infinite looping, so we loop forever even when a finite number is requested
         loopoption = vs.options.loop ≥ 0 ? "loop" : ""
         print(
             io,
-            """<video autoplay controls $loopoption><source src="data:video/x-m4v;base64,""",
+            """<video autoplay controls $loopoption width="$w" height="$h"><source src="data:video/x-m4v;base64,""",
             base64encode(open(read, path)),
             """" type="video/mp4"></video>"""
         )