Don't supply width/height to the device if they aren't defined (#3740)

* Close #1409: don't supply width/height to the device if they aren't defined

* Update news

* Update unit tests to reflect that plotPNG()/startPNG() now handles NULL dimensions

* Add a note about NULL dimensions on plotPNG() help page

* Update news

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)
 
 

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`.

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)
   }
 

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).
+}

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)
 })