diff --git a/NEWS.md b/NEWS.md index af4dc865e..784a466f1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,8 @@ shiny 1.7.3.9000 * Closed #3719: Output container sizes, which are available via [`session$clientData` and `getCurrentOutputInfo()`](https://shiny.rstudio.com/articles/client-data.html), no longer round to the nearest pixel (i.e., they are now more exact, possibly fractional values). (#3720) +* Closed #3704, #3735, and #3740: `renderPlot()` no longer generates an error (or segfault) when it executes before the output is visible. Instead, it'll now use the graphics device's default size for it's initial size. Relatedly, `plotPNG()` now ignores `NULL` values for `width`/`height` (and uses the device's default `width`/`height` instead). (#3739) + ### New features and improvements * `plotOutput()`, `imageOutput()`, and `uiOutput()` gain a `fill` argument. If `TRUE` (the default for `plotOutput()`), the output container is allowed to grow/shrink to fit a fill container (created via `htmltools::bindFillRole()`) with an opinionated height. This means `plotOutput()` will grow/shrink by default [inside of `bslib::card_body_fill()`](https://rstudio.github.io/bslib/articles/cards.html#responsive-sizing), but `imageOutput()` and `uiOutput()` will have to opt-in to similar behavior with `fill = TRUE`. (#3715) @@ -15,8 +17,6 @@ shiny 1.7.3.9000 ### Bug fixes -* Closed #3704 and #3735: `renderPlot()` no longer leads to a segfault when it executes before the output size is known. (#3739) - * Closed #3687: Updated jQuery-UI to v1.13.2. (#3697) diff --git a/R/imageutils.R b/R/imageutils.R index 8ef222c63..aa001794d 100644 --- a/R/imageutils.R +++ b/R/imageutils.R @@ -11,27 +11,13 @@ startPNG <- function(filename, width, height, res, ...) { grDevices::png } - # It's possible for width/height to be NULL (e.g., when using - # suspendWhenHidden=F w/ tabsetPanel()), which will lead to an error when - # attempting to open the device (rstudio/shiny#1409). In this case, ragg will - # actually segfault (instead of error), so explicitly throw an error before - # opening the device (r-lib/ragg#116, rstudio/shiny#3704) - check_empty_png_size <- function(x) { - if (length(x) > 0) return(x) + args <- list2(filename = filename, width = width, height = height, res = res, ...) - rlang::abort( - paste0("Invalid plot `", substitute(x), "`."), - call = rlang::caller_env() - ) - } - - args <- rlang::list2( - filename = filename, - width = check_empty_png_size(width), - height = check_empty_png_size(height), - res = res, - ... - ) + # It's possible for width/height to be NULL/numeric(0) (e.g., when using + # suspendWhenHidden=F w/ tabsetPanel(), see rstudio/shiny#1409), so when + # this happens let the device determine what the default size should be. + if (length(args$width) == 0) args$width <- NULL + if (length(args$height) == 0) args$height <- NULL # Set a smarter default for the device's bg argument (based on thematic's global state). # Note that, technically, this is really only needed for CairoPNG, since the other @@ -84,6 +70,10 @@ startPNG <- function(filename, width, height, res, ...) { #' * Otherwise, use [grDevices::png()]. In this case, Linux and Windows #' may not antialias some point shapes, resulting in poor quality output. #' +#' @details +#' A `NULL` value provided to `width` or `height` is ignored (i.e., the +#' default `width` or `height` of the graphics device is used). +#' #' @param func A function that generates a plot. #' @param filename The name of the output file. Defaults to a temp file with #' extension `.png`. diff --git a/R/render-plot.R b/R/render-plot.R index e0ddc50e7..71e4bda96 100644 --- a/R/render-plot.R +++ b/R/render-plot.R @@ -194,8 +194,8 @@ renderPlot <- function(expr, width = 'auto', height = 'auto', res = 72, ..., } resizeSavedPlot <- function(name, session, result, width, height, alt, pixelratio, res, ...) { - if (result$img$width == width && result$img$height == height && - result$pixelratio == pixelratio && result$res == res) { + if (isTRUE(result$img$width == width && result$img$height == height && + result$pixelratio == pixelratio && result$res == res)) { return(result) } diff --git a/man/plotPNG.Rd b/man/plotPNG.Rd index d925fa3c7..bd2def071 100644 --- a/man/plotPNG.Rd +++ b/man/plotPNG.Rd @@ -46,3 +46,7 @@ is not set to \code{FALSE}), then use \code{\link[Cairo:Cairo]{Cairo::CairoPNG() may not antialias some point shapes, resulting in poor quality output. } } +\details{ +A \code{NULL} value provided to \code{width} or \code{height} is ignored (i.e., the +default \code{width} or \code{height} of the graphics device is used). +} diff --git a/tests/testthat/test-plot-png.R b/tests/testthat/test-plot-png.R index 0740e97f5..500746b18 100644 --- a/tests/testthat/test-plot-png.R +++ b/tests/testthat/test-plot-png.R @@ -1,23 +1,6 @@ -test_that("startPNG() throws informative error", { - tmp <- tempfile(fileext = '.png') - - expect_error( - startPNG( - filename = tmp, - width = NULL, - height = 100, - res = 72 - ), - "Invalid plot `width`." - ) - - expect_error( - startPNG( - filename = tmp, - width = 100, - height = NULL, - res = 72 - ), - "Invalid plot `height`." - ) +test_that("plotPNG()/startPNG() ignores NULL dimensions", { + f <- plotPNG(function() plot(1), width = NULL, height = NULL) + on.exit(unlink(f)) + bits <- readBin(f, "raw", file.info(f)$size) + expect_true(length(bits) > 0) })