Ensure stateless layer instances (#4448)

Author: thomasp85

NEWS.md

@@ -1,14 +1,18 @@
 # ggplot2 (development version)
 
+* Fix a bug in the layer implementation that introduced a new state after the 
+  first render which could lead to a different look when rendered the second 
+  time (@thomasp85, #4204)
+  
 * Make sure `label_bquote()` has access to the calling environment when 
   evaluating the labels (@thomasp85, #4141)
-
+  
 * Fix bug in `annotate_logticks()` that would cause an error when used together
   with `coord_flip()` (@thomasp85, #3954)
 
 * Fix a bug in `guide_bins()` where keys would disappear if the guide was 
   reversed (@thomasp85, #4210)
-
+  
 * Fix a bug in legend justification where justification was lost of the legend
   dimensions exceeded the available size (@thomasp85, #3635)
 

R/layer.r

@@ -169,6 +169,12 @@ Layer <- ggproto("Layer", NULL,
   geom_params = NULL,
   stat = NULL,
   stat_params = NULL,
+
+  # These two fields carry state throughout rendering but will always be
+  # calculated before use
+  computed_geom_params = NULL,
+  computed_stat_params = NULL,
+
   data = NULL,
   aes_params = NULL,
   mapping = NULL,
@@ -276,9 +282,9 @@ Layer <- ggproto("Layer", NULL,
     if (empty(data))
       return(new_data_frame())
 
-    params <- self$stat$setup_params(data, self$stat_params)
-    data <- self$stat$setup_data(data, params)
-    self$stat$compute_layer(data, params, layout)
+    self$computed_stat_params <- self$stat$setup_params(data, self$stat_params)
+    data <- self$stat$setup_data(data, self$computed_stat_params)
+    self$stat$compute_layer(data, self$computed_stat_params, layout)
   },
 
   map_statistic = function(self, data, plot) {
@@ -339,8 +345,8 @@ Layer <- ggproto("Layer", NULL,
       c(names(data), names(self$aes_params)),
       snake_class(self$geom)
     )
-    self$geom_params <- self$geom$setup_params(data, c(self$geom_params, self$aes_params))
-    self$geom$setup_data(data, self$geom_params)
+    self$computed_geom_params <- self$geom$setup_params(data, c(self$geom_params, self$aes_params))
+    self$geom$setup_data(data, self$computed_geom_params)
   },
 
   compute_position = function(self, data, layout) {
@@ -363,7 +369,7 @@ Layer <- ggproto("Layer", NULL,
   },
 
   finish_statistics = function(self, data) {
-    self$stat$finish_layer(data, self$stat_params)
+    self$stat$finish_layer(data, self$computed_stat_params)
   },
 
   draw_geom = function(self, data, layout) {
@@ -372,8 +378,8 @@ Layer <- ggproto("Layer", NULL,
       return(rep(list(zeroGrob()), n))
     }
 
-    data <- self$geom$handle_na(data, self$geom_params)
-    self$geom$draw_layer(data, self$geom_params, layout, layout$coord)
+    data <- self$geom$handle_na(data, self$computed_geom_params)
+    self$geom$draw_layer(data, self$computed_geom_params, layout, layout$coord)
   }
 )
 

tests/testthat/test-layer.r

@@ -63,6 +63,16 @@ test_that("if an aes is mapped to a function that returns NULL, it is removed",
   expect_identical(names(p[[1]]), c("x", "PANEL", "group"))
 })
 
+test_that("layers are stateless except for the computed params", {
+  df <- data.frame(x = 1:10, y = 1:10)
+  p <- ggplot(df) +
+    geom_col(aes(x = x, y = y), width = 0.8, fill = "red")
+  col_layer <- as.list(p$layers[[1]])
+  stateless_names <- setdiff(names(col_layer), c("computed_geom_params", "computed_stat_params"))
+  invisible(ggplotGrob(p))
+  expect_identical(as.list(p$layers[[1]])[stateless_names], col_layer[stateless_names])
+})
+
 # Data extraction ---------------------------------------------------------
 
 test_that("layer_data returns a data.frame", {