Honor width/height/scale when saving plots (#91)

ext/plot_recipes.jl

@@ -328,6 +328,22 @@ function PowerGraphics.save_plot(plot::CairoMakiePlot, filename::String; kwargs.
     )
 end
 
+_save_dimension(::Symbol, ::Nothing) = nothing
+function _save_dimension(name::Symbol, value::Integer)
+    value > 0 ||
+        throw(ArgumentError("`$name` must be a positive integer, got $value"))
+    return Int(value)
+end
+_save_dimension(name::Symbol, value) =
+    throw(ArgumentError("`$name` must be a positive integer, got $value"))
+
+_save_scale(::Nothing) = nothing
+function _save_scale(value::Real)
+    value > 0 || throw(ArgumentError("`scale` must be a positive number, got $value"))
+    return float(value)
+end
+_save_scale(value) = throw(ArgumentError("`scale` must be a positive number, got $value"))
+
 function PowerGraphics.save_plot(
     plot::CairoMakiePlot,
     filename::String,
@@ -344,7 +360,35 @@ function PowerGraphics.save_plot(
             ),
         )
     end
-    CairoMakie.save(filename, plot.figure)
+    width = _save_dimension(:width, get(kwargs, :width, nothing))
+    height = _save_dimension(:height, get(kwargs, :height, nothing))
+    scale = _save_scale(get(kwargs, :scale, nothing))
+
+    # Only the three save-time keywords are forwarded: the caller's kwargs are the
+    # full plot kwarg set, which `CairoMakie.save` would reject or misinterpret.
+    save_kwargs = Dict{Symbol, Any}()
+    if !isnothing(scale)
+        # Each format reads a different resolution knob — png uses `px_per_unit`,
+        # pdf uses `pt_per_unit`, svg uses `pt_per_unit / 0.75` — so scale both
+        # Makie defaults and let each format pick the one it cares about.
+        save_kwargs[:px_per_unit] = 2.0 * scale
+        save_kwargs[:pt_per_unit] = 0.75 * scale
+    end
+
+    scene = plot.figure.scene
+    original_size = size(scene)
+    resized = !isnothing(width) || !isnothing(height)
+    if resized
+        save_kwargs[:size] =
+            (something(width, original_size[1]), something(height, original_size[2]))
+    end
+    try
+        CairoMakie.save(filename, plot.figure; save_kwargs...)
+    finally
+        # `CairoMakie.save` resizes the scene in place and never restores it, so
+        # without this the saved size would leak into the caller's figure.
+        resized && resize!(scene, original_size)
+    end
     @info "saved plot" filename
     return filename
 end

ext/plotly_recipes.jl

@@ -184,8 +184,37 @@ function PowerGraphics._dataframe_plots_internal(
     return plot
 end
 
-const SUPPORTED_PLOTLY_SAVE_KWARGS =
-    [:autoplay, :post_script, :full_html, :animation_opts, :default_width, :default_height]
+_save_dimension(::Symbol, ::Nothing) = nothing
+function _save_dimension(name::Symbol, value::Integer)
+    value > 0 ||
+        throw(ArgumentError("`$name` must be a positive integer, got $value"))
+    return Int(value)
+end
+_save_dimension(name::Symbol, value) =
+    throw(ArgumentError("`$name` must be a positive integer, got $value"))
+
+# Runs `f` with `layout.width`/`layout.height` set, then restores the layout exactly.
+# `EasyConfig.Config` auto-vivifies on read — `layout.width` on a layout without a
+# width *inserts* an empty `Config` that serializes as `"width":{}` — so every read
+# is `haskey`-guarded and absent keys are removed again rather than reset.
+function _with_layout_size(f::Function, plot::PlotlyLight.Plot, width, height)
+    original = Dict{Symbol, Any}(
+        k => plot.layout[k] for k in (:width, :height) if haskey(plot.layout, k)
+    )
+    isnothing(width) || setproperty!(plot.layout, :width, width)
+    isnothing(height) || setproperty!(plot.layout, :height, height)
+    try
+        return f()
+    finally
+        for k in (:width, :height)
+            if haskey(original, k)
+                setproperty!(plot.layout, k, original[k])
+            else
+                delete!(plot.layout, k)
+            end
+        end
+    end
+end
 
 # Two-arg `save_plot` for PlotlyLight plots; inferred from the plot type so
 # callers can write `save_plot(p, "out.html")` and hit the right backend.
@@ -199,27 +228,26 @@ function PowerGraphics.save_plot(plot::PlotlyLight.Plot, filename::String; kwarg
 end
 
 function PowerGraphics.save_plot(
-    plot,
+    plot::PlotlyLight.Plot,
     filename::String,
     backend::PowerGraphics.PlotlyLightBackend;
     kwargs...,
 )
-    save_kwargs =
-        Dict{Symbol, Any}(((k, v) for (k, v) in kwargs if k in SUPPORTED_PLOTLY_SAVE_KWARGS))
-    @info "saving plot" filename
-    if last(splitext(filename)) == ".html"
-        open(filename, "w") do io
-            show(io, MIME("text/html"), plot; save_kwargs...)
-        end
-    else
+    width = _save_dimension(:width, get(kwargs, :width, nothing))
+    height = _save_dimension(:height, get(kwargs, :height, nothing))
+    isnothing(get(kwargs, :scale, nothing)) ||
+        @warn "`scale` is ignored for HTML output; it only applies to the CairoMakie backend."
+    if lowercase(last(splitext(filename))) != ".html"
         # PlotlyLight doesn't have built-in image export
         # Users need to save HTML and convert externally, or use PlotlyBase.jl
         @warn "PlotlyLight only supports HTML export. Saving as HTML instead." filename
-        html_filename = replace(filename, r"\.[^.]+$" => ".html")
-        open(html_filename, "w") do io
-            show(io, MIME("text/html"), plot; save_kwargs...)
+        filename = replace(filename, r"\.[^.]+$" => ".html")
+    end
+    @info "saving plot" filename
+    _with_layout_size(plot, width, height) do
+        open(filename, "w") do io
+            show(io, MIME("text/html"), plot)
         end
-        return html_filename
     end
     return filename
 end

src/call_plots.jl

@@ -837,31 +837,46 @@ end
 end
 
 """
-    save_plot(plot, filename)
+    save_plot(plot, filename; kwargs...)
+    save_plot(plot, filename, backend; kwargs...)
 
-Saves a plot to the specified filename. The backend is chosen from the plot
-object's type: CairoMakie plots dispatch to the CairoMakie writer (png/pdf/svg),
-PlotlyLight plots dispatch to the PlotlyLight writer (html).
+Saves a plot to the specified filename. In the two-argument form the backend is
+chosen from the plot object's type: CairoMakie plots dispatch to the CairoMakie
+writer (png/pdf/svg), PlotlyLight plots dispatch to the PlotlyLight writer (html).
+The three-argument form takes the backend (`CairoMakieBackend()` or
+`PlotlyLightBackend()`) explicitly.
 
 # Arguments
 
 - `plot`: plot object returned by a `plot_*` function
 - `filename::String` : path to save to
+- `backend` : `CairoMakieBackend()` or `PlotlyLightBackend()`, when given explicitly
 
 # Example
 
 ```julia
 res = solve_op_problem!(OpProblem)
 plot = plot_fuel(res)
-save_plot(plot, "my_plot.png")               # CairoMakie
+save_plot(plot, "my_plot.png"; width = 800, height = 600, scale = 2)   # CairoMakie
 plot = plot_fuel_plotly(res)
-save_plot(plot, "my_plot.html")               # PlotlyLight
+save_plot(plot, "my_plot.html"; width = 800, height = 600)             # PlotlyLight
 ```
 
-# Accepted Key Words (PlotlyLight backend only; CairoMakie ignores them)
-- `width::Union{Nothing,Int}=nothing`
-- `height::Union{Nothing,Int}=nothing`
-- `scale::Union{Nothing,Real}=nothing`
+# Accepted Key Words
+
+- `width::Int`, `height::Int`: size of the saved output, in the backend's own units.
+  They affect only the written file — the plot object is left unchanged. CairoMakie
+  reads them as Makie scene units, which the default resolution renders at 2 px each
+  for png (`width = 800` writes a 1600 px wide file) and 0.75 pt each for pdf/svg, and
+  fills a missing dimension from the figure's current size. PlotlyLight writes them to
+  the layout as CSS pixels and leaves a missing dimension to Plotly's own default.
+- `scale::Real`: multiplies CairoMakie's default output resolution — pixel dimensions
+  for png, page extent for pdf/svg. Being relative to those defaults, it overrides a
+  custom `px_per_unit`/`pt_per_unit` theme. CairoMakie only; ignored with a warning for
+  HTML.
+
+These keywords may also be passed to any `plot_*` function, which forwards them to
+`save_plot` when saving with `save`.
 """
 # The 2-arg `save_plot(plot, filename)` form is defined per-backend via type
 # dispatch — see `ext/plot_recipes.jl` (CairoMakie) and `ext/plotly_recipes.jl`

test/test_plot_creation.jl

@@ -1,5 +1,16 @@
 file_path = TEST_OUTPUTS
 
+# Width and height of a PNG, read straight out of the IHDR chunk (big-endian
+# UInt32 at byte offsets 16 and 20), to avoid pulling in an image reader.
+function png_size(filename::String)
+    return open(filename, "r") do io
+        seek(io, 16)
+        width = ntoh(read(io, UInt32))
+        height = ntoh(read(io, UInt32))
+        (Int(width), Int(height))
+    end
+end
+
 function test_plots(file_path::String; backend_pkg::String = "cairomakie")
     # Select plot functions based on backend
     if backend_pkg == "cairomakie"
@@ -377,6 +388,117 @@ function test_plots(file_path::String; backend_pkg::String = "cairomakie")
         cleanup && rm(out_path; recursive = true)
     end
 
+    @testset "test $backend_pkg save sizing" begin
+        out_path = joinpath(file_path, backend_pkg * "_save_sizing")
+        !isdir(out_path) && mkdir(out_path)
+        p = plot_dataframe_fn(
+            gen_uc.data[:ActivePowerVariable__ThermalStandard],
+            gen_uc.time;
+            set_display = false,
+            title = "sizing",
+            stack = true,
+        )
+
+        if backend_pkg == "cairomakie"
+            original_size = size(p.figure.scene)
+
+            default_png = joinpath(out_path, "default.png")
+            PG.save_plot(p, default_png)
+            # The 1280x720 default figure at Makie's default px_per_unit of 2.
+            @test png_size(default_png) == (2560, 1440)
+
+            sized_png = joinpath(out_path, "sized.png")
+            PG.save_plot(p, sized_png; width = 800, height = 600)
+            @test png_size(sized_png) == (1600, 1200)
+
+            scaled_png = joinpath(out_path, "scaled.png")
+            PG.save_plot(p, scaled_png; width = 800, height = 600, scale = 2)
+            @test png_size(scaled_png) == (3200, 2400)
+
+            # Saving at a different size must never mutate the caller's figure.
+            @test size(p.figure.scene) == original_size
+
+            # Save-time sizing is independent of the plot-time `size` kwarg, which
+            # `save_plot` must ignore even though `plot_*` forwards it here.
+            ignores_size = joinpath(out_path, "ignores_size.png")
+            PG.save_plot(p, ignores_size; size = (300, 200))
+            @test png_size(ignores_size) == (2560, 1440)
+
+            # `CairoMakie.save` resizes the scene before it writes, so a failure
+            # mid-write must still leave the figure at its original size.
+            @test_throws Exception PG.save_plot(
+                p,
+                joinpath(out_path, "missing_dir", "x.png");
+                width = 321,
+                height = 123,
+            )
+            @test size(p.figure.scene) == original_size
+
+            for format in ("svg", "pdf")
+                vector_file = joinpath(out_path, "vector.$format")
+                PG.save_plot(p, vector_file; width = 400, height = 300, scale = 1.5)
+                @test filesize(vector_file) > 0
+            end
+            @test size(p.figure.scene) == original_size
+
+            @test_throws ArgumentError PG.save_plot(p, joinpath(out_path, "bad.html"))
+            @test_throws ArgumentError PG.save_plot(
+                p,
+                joinpath(out_path, "bad.png");
+                width = 0,
+            )
+            @test_throws ArgumentError PG.save_plot(
+                p,
+                joinpath(out_path, "bad.png");
+                scale = -1,
+            )
+        else
+            @test !haskey(p.layout, :width)
+
+            sized_html = joinpath(out_path, "sized.html")
+            PG.save_plot(p, sized_html; width = 640, height = 480)
+            html = read(sized_html, String)
+            @test occursin("\"width\":640", html)
+            @test occursin("\"height\":480", html)
+            # An unguarded `layout.width` read would serialize as `"width":{}`.
+            @test !occursin("\"width\":{}", html)
+            # The layout must be left exactly as it was found.
+            @test !haskey(p.layout, :width)
+            @test !haskey(p.layout, :height)
+
+            # Keywords `show` cannot accept must be dropped, not forwarded.
+            extra_html = joinpath(out_path, "extra.html")
+            PG.save_plot(p, extra_html; full_html = true)
+            @test filesize(extra_html) > 0
+
+            @test_logs (:warn, r"scale") match_mode = :any PG.save_plot(
+                p,
+                joinpath(out_path, "scaled.html");
+                scale = 2,
+            )
+
+            # Non-HTML requests fall back to HTML and report the name written.
+            written = @test_logs (:warn, r"HTML") match_mode = :any PG.save_plot(
+                p,
+                joinpath(out_path, "raster.png"),
+            )
+            @test written == joinpath(out_path, "raster.html")
+            @test isfile(written)
+
+            # A failed write must still restore the layout it borrowed.
+            @test_throws Exception PG.save_plot(
+                p,
+                joinpath(out_path, "missing_dir", "x.html");
+                width = 640,
+                height = 480,
+            )
+            @test !haskey(p.layout, :width)
+            @test !haskey(p.layout, :height)
+        end
+
+        cleanup && rm(out_path; recursive = true)
+    end
+
     # HTML saving only works with PlotlyLight backend
     if backend_pkg == "plotlylight"
         @testset "test html saving" begin