From aa577eceaf62b9415010af006aa2deb042539f2e Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 10:21:50 +0200
Subject: [PATCH 01/17] isolate wrap panel initialisation

---
 R/facet-wrap.R | 69 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 720e2e8e37..aad0b8e5eb 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -257,6 +257,42 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     data$PANEL <- layout$PANEL[match(keys$x, keys$y)]
     data
   },
+
+  init_panels = function(panels, layout, theme, aspect_ratio = NULL, clip = "on") {
+
+    dim <- c(max(layout$ROW), max(layout$COL))
+
+    # Initialise matrix of panels
+    table <- matrix(list(zeroGrob()), dim[1], dim[2])
+    table[cbind(layout$ROW, layout$COL)] <- panels
+
+    table <- gtable_matrix(
+      "layout", table,
+      widths  = unit(rep(1, dim[2]), "null"),
+      heights = unit(rep(aspect_ratio %||% 1, dim[1]), "null"),
+      respect = !is.null(aspect_ratio),
+      clip = clip,
+      z = matrix(1, dim[1], dim[2])
+    )
+
+    # Set panel names
+    table$layout$name <- paste(
+      "panel",
+      rep(seq_len(dim[2]), dim[1]),
+      rep(seq_len(dim[1]), each = dim[2]),
+      sep = "-"
+    )
+
+    # Add panel spacing
+    spacing <- lapply(
+      c(x = "panel.spacing.x", y = "panel.spacing.y"),
+      calc_element, theme = theme
+    )
+    table <- gtable_add_col_space(table, spacing$x)
+    table <- gtable_add_row_space(table, spacing$y)
+    table
+  },
+
   draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
       cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}.")
@@ -281,6 +317,15 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     panel_order <- order(layout$ROW, layout$COL)
     layout <- layout[panel_order, ]
     panels <- panels[panel_order]
+
+    panel_table <- self$init_panels(
+      panels, layout, theme,
+      # If user hasn't set aspect ratio, ask the coordinate system if
+      # it wants to specify one
+      aspect_ratio = theme$aspect.ratio %||% coord$aspect(ranges[[1]]),
+      clip = coord$clip
+    )
+
     panel_pos <- convertInd(layout$ROW, layout$COL, nrow)
 
     # Fill missing parameters for backward compatibility
@@ -305,29 +350,9 @@ FacetWrap <- ggproto("FacetWrap", Facet,
       structure(labels_df, type = "cols"),
       params$labeller, theme)
 
-    # If user hasn't set aspect ratio, ask the coordinate system if
-    # it wants to specify one
-    aspect_ratio <- theme$aspect.ratio %||% coord$aspect(ranges[[1]])
-
-    if (is.null(aspect_ratio)) {
-      aspect_ratio <- 1
-      respect <- FALSE
-    } else {
-      respect <- TRUE
-    }
-
     empty_table <- matrix(list(zeroGrob()), nrow = nrow, ncol = ncol)
-    panel_table <- empty_table
-    panel_table[panel_pos] <- panels
-    empties <- apply(panel_table, c(1,2), function(x) is.zero(x[[1]]))
-    panel_table <- gtable_matrix("layout", panel_table,
-     widths = unit(rep(1, ncol), "null"),
-     heights = unit(rep(abs(aspect_ratio), nrow), "null"), respect = respect, clip = coord$clip, z = matrix(1, ncol = ncol, nrow = nrow))
-    panel_table$layout$name <- paste0('panel-', rep(seq_len(ncol), nrow), '-', rep(seq_len(nrow), each = ncol))
-
-
-    panel_table <- gtable_add_col_space(panel_table, calc_element("panel.spacing.x", theme))
-    panel_table <- gtable_add_row_space(panel_table, calc_element("panel.spacing.y", theme))
+    empties <- matrix(TRUE, nrow, ncol)
+    empties[cbind(layout$ROW, layout$COL)] <- vapply(panels, is.zero, logical(1))
 
     # Add axes
     axis_mat_x_top <- empty_table

From 9f29bb96ee4add43367857cc0ed2a5603cfc022f Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 12:21:45 +0200
Subject: [PATCH 02/17] isolate wrap axis attachment

---
 R/facet-wrap.R | 235 ++++++++++++++++++++++++-------------------------
 1 file changed, 114 insertions(+), 121 deletions(-)

diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index aad0b8e5eb..67ea138b1c 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -293,6 +293,119 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     table
   },
 
+  attach_axes = function(table, layout, ranges, coord, theme, params) {
+
+    # Setup parameters
+    draw_axes   <- params$draw_axes   %||% list(x = FALSE, y = FALSE)
+    axis_labels <- params$axis_labels %||% list(x = TRUE,  y = TRUE)
+    free        <- params$free        %||% list(x = FALSE, y = FALSE)
+
+    # Render individual axes
+    ranges <- censor_labels(ranges, layout, axis_labels)
+    original   <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
+
+    # Sort axes
+    x_order <- if (axis_labels$x) layout$SCALE_X else seq_len(nrow(layout))
+    y_order <- if (axis_labels$y) layout$SCALE_Y else seq_len(nrow(layout))
+    original$x <- lapply(original$x, `[`, i = x_order)
+    original$y <- lapply(original$y, `[`, i = y_order)
+
+    # Setup matrices for axes
+    dim <- c(max(layout$ROW), max(layout$COL))
+    index <- convertInd(layout$ROW, layout$COL, dim[1])
+    empty <- matrix(list(zeroGrob()), dim[1], dim[2])
+    top <- bottom <- left <- right <- empty
+
+    # Fill axis matrices
+    top[index]    <- original$x$top
+    bottom[index] <- original$x$bottom
+    left[index]   <- original$y$left
+    right[index]  <- original$y$right
+
+    # Suppress interior axes
+    if (!(free$x || draw_axes$x)) {
+      top[-1, ]         <- list(zeroGrob())
+      bottom[-dim[1], ] <- list(zeroGrob())
+    }
+    if (!(free$y || draw_axes$y)) {
+      left[, -1]       <- list(zeroGrob())
+      right[, -dim[2]] <- list(zeroGrob())
+    }
+
+    # Check for empty panels and exit early if there are none
+    empty <- matrix(TRUE, dim[1], dim[2])
+    empty[index] <- FALSE
+    if (!any(empty)) {
+      axes <- list(top = top, bottom = bottom, left = left, right = right)
+      return(weave_axes(table, axes, empty))
+    }
+
+    # Match empty table to layout
+    matched <- vec_match(
+      data_frame0(ROW = as.vector(row(empty)), COL = as.vector(col(empty))),
+      layout[, c("ROW", "COL")]
+    )
+
+    # Figure out where axes should be added back
+    empty_bottom <- which(  apply(empty, 2, function(x) c(diff(x) == 1, FALSE)))
+    empty_top    <- which(  apply(empty, 2, function(x) c(FALSE, diff(x) == -1)))
+    empty_right  <- which(t(apply(empty, 1, function(x) c(diff(x) == 1, FALSE))))
+    empty_left   <- which(t(apply(empty, 1, function(x) c(FALSE, diff(x) == -1))))
+
+    # Keep track of potential clashes between strips and axes
+    inside <- (theme$strip.placement %||% "inside") == "inside"
+    strip  <- params$strip.position %||% "top"
+    clash  <- c(top = FALSE, bottom = FALSE, left = FALSE, right = FALSE)
+
+    # Go through every position and place back axes
+    if (length(empty_bottom) > 0) {
+      x_axes <- original$x$bottom[matched[empty_bottom]]
+      clash["bottom"] <- strip == "bottom" && !inside && !free$x &&
+        !all(vapply(x_axes, is.zero, logical(1)))
+      if (!clash["bottom"]) {
+        bottom[empty_bottom] <- x_axes
+      }
+    }
+
+    if (length(empty_top) > 0) {
+      x_axes <- original$x$top[matched[empty_top]]
+      clash["top"] <- strip == "top" && !inside && !free$x &&
+        !all(vapply(x_axes, is.zero, logical(1)))
+      if (!clash["top"]) {
+        top[empty_top] <- x_axes
+      }
+    }
+
+    if (length(empty_right) > 0) {
+      y_axes <- original$y$right[matched[empty_right]]
+      clash["right"]  <- strip == "right" && !inside && !free$y &&
+        !all(vapply(y_axes, is.zero, logical(1)))
+      if (!clash["right"]) {
+        right[empty_right] <- y_axes
+      }
+    }
+
+    if (length(empty_left) > 0) {
+      y_axes <- original$y$left[matched[empty_left]]
+      clash["left"]  <- strip == "left" && !inside && !free$y &&
+        !all(vapply(y_axes, is.zero, logical(1)))
+      if (!clash["left"]) {
+        left[empty_left] <- y_axes
+      }
+    }
+
+    if (any(clash)) {
+      cli::cli_warn(
+        "Suppressing axis rendering when \\
+        {.code strip.position =\"{strip}\"} and \\
+        {.code strip.placement = \"outside\".}"
+      )
+    }
+
+    axes <- list(top = top, bottom = bottom, left = left, right = right)
+    weave_axes(table, axes, empty)
+  },
+
   draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
       cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}.")
@@ -328,16 +441,6 @@ FacetWrap <- ggproto("FacetWrap", Facet,
 
     panel_pos <- convertInd(layout$ROW, layout$COL, nrow)
 
-    # Fill missing parameters for backward compatibility
-    params$draw_axes   <- params$draw_axes   %||% list(x = FALSE, y = FALSE)
-    params$axis_labels <- params$axis_labels %||% list(x = TRUE,  y = TRUE)
-
-    x_axis_order <- if (params$axis_labels$x) layout$SCALE_X else seq(n)
-    y_axis_order <- if (params$axis_labels$y) layout$SCALE_Y else seq(n)
-
-    ranges <- censor_labels(ranges, layout, params$axis_labels)
-    axes <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
-
     if (length(params$facets) == 0) {
       # Add a dummy label
       labels_df <- data_frame0("(all)" = "(all)", .size = 1)
@@ -351,118 +454,8 @@ FacetWrap <- ggproto("FacetWrap", Facet,
       params$labeller, theme)
 
     empty_table <- matrix(list(zeroGrob()), nrow = nrow, ncol = ncol)
-    empties <- matrix(TRUE, nrow, ncol)
-    empties[cbind(layout$ROW, layout$COL)] <- vapply(panels, is.zero, logical(1))
-
-    # Add axes
-    axis_mat_x_top <- empty_table
-    axis_mat_x_top[panel_pos] <- axes$x$top[x_axis_order]
-    axis_mat_x_bottom <- empty_table
-    axis_mat_x_bottom[panel_pos] <- axes$x$bottom[x_axis_order]
-    axis_mat_y_left <- empty_table
-    axis_mat_y_left[panel_pos] <- axes$y$left[y_axis_order]
-    axis_mat_y_right <- empty_table
-    axis_mat_y_right[panel_pos] <- axes$y$right[y_axis_order]
-    if (!(params$free$x || params$draw_axes$x)) {
-      axis_mat_x_top[-1,]<- list(zeroGrob())
-      axis_mat_x_bottom[-nrow,]<- list(zeroGrob())
-    }
-    if (!(params$free$y || params$draw_axes$y)) {
-      axis_mat_y_left[, -1] <- list(zeroGrob())
-      axis_mat_y_right[, -ncol] <- list(zeroGrob())
-    }
 
-
-    # Add back missing axes
-    if (any(empties)) {
-      row_ind <- row(empties)
-      col_ind <- col(empties)
-      inside <- (theme$strip.placement %||% "inside") == "inside"
-      empty_bottom <- apply(empties, 2, function(x) c(diff(x) == 1, FALSE))
-      if (any(empty_bottom)) {
-        pos <- which(empty_bottom)
-        panel_loc <- data_frame0(
-          ROW = row_ind[pos],
-          COL = col_ind[pos],
-          .size = length(pos)
-        )
-        panels <- vec_match(panel_loc, layout[, c("ROW", "COL")])
-        x_axes <- axes$x$bottom[x_axis_order[panels]]
-        if (params$strip.position == "bottom" &&
-            !inside &&
-            any(!vapply(x_axes, is.zero, logical(1))) &&
-            !params$free$x) {
-          cli::cli_warn("Suppressing axis rendering when {.code strip.position = \"bottom\"} and {.code strip.placement == \"outside\"}")
-        } else {
-          axis_mat_x_bottom[pos] <- x_axes
-        }
-      }
-      empty_top <- apply(empties, 2, function(x) c(FALSE, diff(x) == -1))
-      if (any(empty_top)) {
-        pos <- which(empty_top)
-        panel_loc <- data_frame0(
-          ROW = row_ind[pos],
-          COL = col_ind[pos],
-          .size = length(pos)
-        )
-        panels <- vec_match(panel_loc, layout[, c("ROW", "COL")])
-        x_axes <- axes$x$top[x_axis_order[panels]]
-        if (params$strip.position == "top" &&
-            !inside &&
-            any(!vapply(x_axes, is.zero, logical(1))) &&
-            !params$free$x) {
-          cli::cli_warn("Suppressing axis rendering when {.code strip.position = \"top\"} and {.code strip.placement == \"outside\"}")
-        } else {
-          axis_mat_x_top[pos] <- x_axes
-        }
-      }
-      empty_right <- t(apply(empties, 1, function(x) c(diff(x) == 1, FALSE)))
-      if (any(empty_right)) {
-        pos <- which(empty_right)
-        panel_loc <- data_frame0(
-          ROW = row_ind[pos],
-          COL = col_ind[pos],
-          .size = length(pos)
-        )
-        panels <- vec_match(panel_loc, layout[, c("ROW", "COL")])
-        y_axes <- axes$y$right[y_axis_order[panels]]
-        if (params$strip.position == "right" &&
-            !inside &&
-            any(!vapply(y_axes, is.zero, logical(1))) &&
-            !params$free$y) {
-          cli::cli_warn("Suppressing axis rendering when {.code strip.position = \"right\"} and {.code strip.placement == \"outside\"}")
-        } else {
-          axis_mat_y_right[pos] <- y_axes
-        }
-      }
-      empty_left <- t(apply(empties, 1, function(x) c(FALSE, diff(x) == -1)))
-      if (any(empty_left)) {
-        pos <- which(empty_left)
-        panel_loc <- data_frame0(
-          ROW = row_ind[pos],
-          COL = col_ind[pos],
-          .size = length(pos)
-        )
-        panels <- vec_match(panel_loc, layout[, c("ROW", "COL")])
-        y_axes <- axes$y$left[y_axis_order[panels]]
-        if (params$strip.position == "left" &&
-            !inside &&
-            any(!vapply(y_axes, is.zero, logical(1))) &&
-            !params$free$y) {
-          cli::cli_warn("Suppressing axis rendering when {.code strip.position = \"left\"} and {.code strip.placement == \"outside\"}")
-        } else {
-          axis_mat_y_left[pos] <- y_axes
-        }
-      }
-    }
-    panel_table <- weave_axes(
-      panel_table,
-      axes = list(
-        top  = axis_mat_x_top,  bottom = axis_mat_x_bottom,
-        left = axis_mat_y_left, right  = axis_mat_y_right
-      ),
-      empty = empties
-    )
+    panel_table <- self$attach_axes(panel_table, layout, ranges, coord, theme, params)
     axis_size   <- panel_table$sizes
     panel_table <- panel_table$panels
 

From 44b46079536e66b1410265230172f8035c8b76d3 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 13:16:22 +0200
Subject: [PATCH 03/17] isolate wrap strip attachment

---
 R/facet-wrap.R | 116 ++++++++++++++++++++++++-------------------------
 1 file changed, 57 insertions(+), 59 deletions(-)

diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 67ea138b1c..3caff411da 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -406,6 +406,61 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     weave_axes(table, axes, empty)
   },
 
+  attach_strips = function(table, layout, axis_size, params, theme) {
+
+    # Format labels
+    if (length(params$facets) == 0) {
+      labels <- data_frame0("(all)" = "(all)", .size = 1)
+    } else {
+      labels <- layout[names(params$facets)]
+    }
+    attr(labels, "facet") <- "wrap"
+
+    # Render individual strips
+    strips <- render_strips(
+      x = structure(labels, type = "rows"),
+      y = structure(labels, type = "cols"),
+      params$labeller, theme
+    )
+
+    # Set position invariant parameters
+    padding  <- convertUnit(calc_element("strip.switch.pad.wrap", theme), "cm")
+    position <- params$strip.position %||% "top"
+    prefix   <- paste0("strip-", substr(position, 1, 1))
+    pad <- axis_size[[position]]
+
+    # Setup weaving table
+    dim <- c(max(layout$ROW), max(layout$COL))
+    index <- convertInd(layout$ROW, layout$COL, dim[1])
+    mat <- matrix(list(zeroGrob()), dim[1], dim[2])
+    mat[index] <- unlist(unname(strips), recursive = FALSE)[[position]]
+
+    # Setup orientation dependent parameters
+    if (position %in% c("top", "bottom")) {
+      inside  <- "strip.placement.x"
+      size    <- apply(mat, 1, max_height, value_only = TRUE)
+      weave   <- weave_tables_row
+    } else {
+      inside  <- "strip.placement.y"
+      size    <- apply(mat, 2, max_width, value_only = TRUE)
+      weave   <- weave_tables_col
+      pad[as.numeric(pad) != 0] <- padding
+    }
+
+    inside <- (calc_element(inside, theme) %||% "inside") == "inside"
+    shift  <- switch(position, top = , left = c(-1, 2), c(0, 1))
+    shift  <- if (inside) shift[1] else shift[2]
+    size   <- unit(size, "cm")
+
+    table <- weave(table, mat, shift, size, name = prefix, z = 2, clip = "on")
+    if (!inside) {
+      pad[as.numeric(pad) != 0] <- padding
+      table <- weave(table, missing_arg(), shift, pad)
+    }
+
+    table
+  },
+
   draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
       cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}.")
@@ -424,9 +479,6 @@ FacetWrap <- ggproto("FacetWrap", Facet,
       }
     }
 
-    ncol <- max(layout$COL)
-    nrow <- max(layout$ROW)
-    n <- nrow(layout)
     panel_order <- order(layout$ROW, layout$COL)
     layout <- layout[panel_order, ]
     panels <- panels[panel_order]
@@ -439,63 +491,9 @@ FacetWrap <- ggproto("FacetWrap", Facet,
       clip = coord$clip
     )
 
-    panel_pos <- convertInd(layout$ROW, layout$COL, nrow)
-
-    if (length(params$facets) == 0) {
-      # Add a dummy label
-      labels_df <- data_frame0("(all)" = "(all)", .size = 1)
-    } else {
-      labels_df <- layout[names(params$facets)]
-    }
-    attr(labels_df, "facet") <- "wrap"
-    strips <- render_strips(
-      structure(labels_df, type = "rows"),
-      structure(labels_df, type = "cols"),
-      params$labeller, theme)
-
-    empty_table <- matrix(list(zeroGrob()), nrow = nrow, ncol = ncol)
-
     panel_table <- self$attach_axes(panel_table, layout, ranges, coord, theme, params)
-    axis_size   <- panel_table$sizes
-    panel_table <- panel_table$panels
-
-    strip_padding <- convertUnit(calc_element("strip.switch.pad.wrap", theme), "cm")
-    strip_name <- paste0("strip-", substr(params$strip.position, 1, 1))
-    strip_mat <- empty_table
-    strip_mat[panel_pos] <- unlist(unname(strips), recursive = FALSE)[[params$strip.position]]
-    if (params$strip.position %in% c("top", "bottom")) {
-      inside_x <- (theme$strip.placement.x %||% theme$strip.placement %||% "inside") == "inside"
-      if (params$strip.position == "top") {
-        placement <- if (inside_x) -1 else -2
-        strip_pad <- axis_size$top
-      } else {
-        placement <- if (inside_x) 0 else 1
-        strip_pad <- axis_size$bottom
-      }
-      strip_height <- unit(apply(strip_mat, 1, max_height, value_only = TRUE), "cm")
-      panel_table <- weave_tables_row(panel_table, strip_mat, placement, strip_height, strip_name, 2, coord$clip)
-      if (!inside_x) {
-        strip_pad[as.numeric(strip_pad) != 0] <- strip_padding
-        panel_table <- weave_tables_row(panel_table, row_shift = placement, row_height = strip_pad)
-      }
-    } else {
-      inside_y <- (theme$strip.placement.y %||% theme$strip.placement %||% "inside") == "inside"
-      if (params$strip.position == "left") {
-        placement <- if (inside_y) -1 else -2
-        strip_pad <- axis_size$left
-      } else {
-        placement <- if (inside_y) 0 else 1
-        strip_pad <- axis_size$right
-      }
-      strip_pad[as.numeric(strip_pad) != 0] <- strip_padding
-      strip_width <- unit(apply(strip_mat, 2, max_width, value_only = TRUE), "cm")
-      panel_table <- weave_tables_col(panel_table, strip_mat, placement, strip_width, strip_name, 2, coord$clip)
-      if (!inside_y) {
-        strip_pad[as.numeric(strip_pad) != 0] <- strip_padding
-        panel_table <- weave_tables_col(panel_table, col_shift = placement, col_width = strip_pad)
-      }
-    }
-    panel_table
+
+    self$attach_strips(panel_table$panels, layout, panel_table$sizes, params, theme)
   },
   vars = function(self) {
     names(self$params$facets)

From a1fc986d869922d444190e664f8d1c6e12bd74e0 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 13:49:35 +0200
Subject: [PATCH 04/17] isolate grid panel initialisation

---
 R/facet-grid-.R | 93 ++++++++++++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 39 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 0854b5299b..8e7745f011 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -335,7 +335,54 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     }
     data
   },
-  draw_panels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
+
+  init_panels = function(panels, layout, theme, ranges, params, aspect_ratio, clip = "on") {
+
+    dim   <- c(max(layout$ROW), max(layout$COL))
+    table <- matrix(panels, dim[1], dim[2], byrow = TRUE)
+
+    space <- params$space_free %||% list(x = FALSE, y = FALSE)
+
+    widths  <- unit(rep(1, dim[2]), "null")
+    heights <- unit(rep(1 * abs(aspect_ratio %||% 1), dim[1]), "null")
+
+    if (space$x) {
+      idx    <- layout$PANEL[layout$ROW == 1]
+      widths <- vapply(idx, function(i) diff(ranges[[i]]$x.range), numeric(1))
+      widths <- unit(widths, "null")
+    }
+
+    if (space$y) {
+      idx <- layout$PANEL[layout$COL == 1]
+      heights <- vapply(idx, function(i) diff(ranges[[i]]$y.range), numeric(1))
+      heights <- unit(heights, "null")
+    }
+
+    table <- gtable_matrix(
+      "layout", table,
+      widths = widths, heights = heights,
+      respect = !is.null(aspect_ratio),
+      clip = clip, z = matrix(1, dim[1], dim[2])
+    )
+
+    table$layout$name <- paste(
+      "panel",
+      rep(seq_len(dim[2]), dim[1]),
+      rep(seq_len(dim[1]), each = dim[2]),
+      sep = "-"
+    )
+
+    spacing <- lapply(
+      c(x = "panel.spacing.x", y = "panel.spacing.y"),
+      calc_element, theme = theme
+    )
+
+    table <- gtable_add_col_space(table, spacing$x)
+    table <- gtable_add_row_space(table, spacing$y)
+    table
+  },
+
+  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
       cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
     }
@@ -375,47 +422,15 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     if (!is.null(aspect_ratio) && (params$space_free$x || params$space_free$y)) {
       cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
     }
-    aspect_ratio <- aspect_ratio %||% coord$aspect(ranges[[1]])
-    if (is.null(aspect_ratio)) {
-      aspect_ratio <- 1
-      respect <- FALSE
-    } else {
-      respect <- TRUE
-    }
+
+    panel_table <- self$init_panels(
+      panels, layout, theme, ranges, params,
+      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]])
+    )
+
     ncol <- max(layout$COL)
     nrow <- max(layout$ROW)
     mtx <- function(x) matrix(x, nrow = nrow, ncol = ncol, byrow = TRUE)
-    panel_table <- mtx(panels)
-
-    # @kohske
-    # Now size of each panel is calculated using PANEL$ranges, which is given by
-    # coord_train called by train_range.
-    # So here, "scale" need not to be referred.
-    #
-    # In general, panel has all information for building facet.
-    if (params$space_free$x) {
-      ps <- layout$PANEL[layout$ROW == 1]
-      widths <- vapply(ps, function(i) diff(ranges[[i]]$x.range), numeric(1))
-      panel_widths <- unit(widths, "null")
-    } else {
-      panel_widths <- rep(unit(1, "null"), ncol)
-    }
-    if (params$space_free$y) {
-      ps <- layout$PANEL[layout$COL == 1]
-      heights <- vapply(ps, function(i) diff(ranges[[i]]$y.range), numeric(1))
-      panel_heights <- unit(heights, "null")
-    } else {
-      panel_heights <- rep(unit(1 * abs(aspect_ratio), "null"), nrow)
-    }
-
-    panel_table <- gtable_matrix("layout", panel_table,
-      panel_widths, panel_heights, respect = respect, clip = coord$clip, z = mtx(1))
-    panel_table$layout$name <- paste0('panel-', rep(seq_len(nrow), ncol), '-', rep(seq_len(ncol), each = nrow))
-
-    spacing_x <- calc_element("panel.spacing.x", theme)
-    spacing_y <- calc_element("panel.spacing.y", theme)
-    panel_table <- gtable_add_col_space(panel_table, spacing_x)
-    panel_table <- gtable_add_row_space(panel_table, spacing_y)
 
     # Add axes
     if (params$draw_axes$x) {

From 4dc3b44632a58d57a56090cbb505b6c117402d94 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 14:40:19 +0200
Subject: [PATCH 05/17] isolate grid axis attachment

---
 R/facet-grid-.R | 111 ++++++++++++++++++++++++++++++------------------
 1 file changed, 70 insertions(+), 41 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 8e7745f011..dfdf7b0e52 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -382,32 +382,80 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     table
   },
 
-  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
+  attach_axes = function(table, layout, ranges, coord, theme, params) {
+
+    dim <- c(max(layout$ROW), max(layout$COL))
+
+    draw_axes   <- params$draw_axes   %||% list(x = FALSE, y = FALSE)
+    axis_labels <- params$axis_labels %||% list(x = TRUE,  y = TRUE)
+
+    if (!axis_labels$x) {
+      cols    <- seq_len(nrow(layout))
+      x_order <- as.integer(layout$PANEL[order(layout$ROW, layout$COL)])
+    } else {
+      cols    <- which(layout$ROW == 1)
+      x_order <- layout$COL
+    }
+    if (!axis_labels$y) {
+      rows    <- seq_len(nrow(layout))
+      y_order <- as.integer(layout$PANEL[order(layout$ROW, layout$COL)])
+    } else {
+      rows    <- which(layout$COL == 1)
+      y_order <- layout$ROW
     }
 
-    # Fill missing parameters for backward compatibility
-    params$draw_axes   <- params$draw_axes   %||% list(x = FALSE, y = FALSE)
-    params$axis_labels <- params$axis_labels %||% list(x = TRUE,  y = TRUE)
+    ranges <- censor_labels(ranges, layout, axis_labels)
+    axes   <- render_axes(ranges[cols], ranges[rows], coord, theme, transpose = TRUE)
 
-    if (!params$axis_labels$x) {
-      cols <- seq_len(nrow(layout))
-      x_axis_order <- as.integer(layout$PANEL[order(layout$ROW, layout$COL)])
+    if (draw_axes$x) {
+      table <- weave_axes(
+        table,
+        lapply(axes$x, function(x) matrix(x[x_order], dim[1], dim[2], byrow = TRUE))
+      )$panels
     } else {
-      cols <- which(layout$ROW == 1)
-      x_axis_order <- layout$COL
+      table  <- gtable_add_rows(table, max_height(axes$x$top), 0)
+      table  <- gtable_add_rows(table, max_height(axes$x$bottom), -1)
+      panels <- panel_cols(table)
+      table  <- gtable_add_grob(
+        table, axes$x$top,
+        t = 1, l = panels$l, clip = "off", z = 3,
+        name = paste0("axis-t-", seq_along(axes$x$top))
+      )
+      table <- gtable_add_grob(
+        table, axes$x$bottom,
+        t = -1, l = panels$l, clip = "off", z = 3,
+        name = paste0("axis-b-", seq_along(axes$x$bottom))
+      )
     }
-    if (!params$axis_labels$y) {
-      rows <- seq_len(nrow(layout))
-      y_axis_order <- as.integer(layout$PANEL[order(layout$ROW, layout$COL)])
+
+    if (draw_axes$y) {
+      table <- weave_axes(
+        table,
+        lapply(axes$y, function(y) matrix(y[y_order], dim[1], dim[2], byrow = TRUE))
+      )$panels
     } else {
-      rows <- which(layout$COL == 1)
-      y_axis_order <- layout$ROW
+      table  <- gtable_add_cols(table, max_width(axes$y$left), 0)
+      table  <- gtable_add_cols(table, max_width(axes$y$right), -1)
+      panels <- panel_rows(table)
+      table  <- gtable_add_grob(
+        table, axes$y$left,
+        t = panels$t, l = 1, clip = "off", z = 3,
+        name = paste0("axis-l-", seq_along(axes$y$left))
+      )
+      table <- gtable_add_grob(
+        table, axes$y$right,
+        t = panels$t, l = -1, clip = "off", z = 3,
+        name = paste0("axis-r-", seq_along(axes$y$right))
+      )
     }
 
-    ranges <- censor_labels(ranges, layout, params$axis_labels)
-    axes <- render_axes(ranges[cols], ranges[rows], coord, theme, transpose = TRUE)
+    table
+  },
+
+  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
+    if ((params$free$x || params$free$y) && !coord$is_free()) {
+      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
+    }
 
     col_vars <- unique0(layout[names(params$cols)])
     row_vars <- unique0(layout[names(params$rows)])
@@ -428,33 +476,14 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]])
     )
 
+    panel_table <- self$attach_axes(
+      panel_table, layout, ranges, coord, theme, params
+    )
+
     ncol <- max(layout$COL)
     nrow <- max(layout$ROW)
     mtx <- function(x) matrix(x, nrow = nrow, ncol = ncol, byrow = TRUE)
 
-    # Add axes
-    if (params$draw_axes$x) {
-      axes$x <- lapply(axes$x, function(x) mtx(x[x_axis_order]))
-      panel_table <- weave_axes(panel_table, axes$x)$panels
-    } else {
-      panel_table <- gtable_add_rows(panel_table, max_height(axes$x$top), 0)
-      panel_table <- gtable_add_rows(panel_table, max_height(axes$x$bottom), -1)
-      panel_pos_col <- panel_cols(panel_table)
-      panel_table <- gtable_add_grob(panel_table, axes$x$top, 1, panel_pos_col$l, clip = "off", name = paste0("axis-t-", seq_along(axes$x$top)), z = 3)
-      panel_table <- gtable_add_grob(panel_table, axes$x$bottom, -1, panel_pos_col$l, clip = "off", name = paste0("axis-b-", seq_along(axes$x$bottom)), z = 3)
-    }
-
-    if (params$draw_axes$y) {
-      axes$y <- lapply(axes$y, function(y) mtx(y[y_axis_order]))
-      panel_table <- weave_axes(panel_table, axes$y)$panels
-    } else {
-      panel_table <- gtable_add_cols(panel_table, max_width(axes$y$left), 0)
-      panel_table <- gtable_add_cols(panel_table, max_width(axes$y$right), -1)
-      panel_pos_rows <- panel_rows(panel_table)
-      panel_table <- gtable_add_grob(panel_table, axes$y$left, panel_pos_rows$t, 1, clip = "off", name = paste0("axis-l-", seq_along(axes$y$left)), z = 3)
-      panel_table <- gtable_add_grob(panel_table, axes$y$right, panel_pos_rows$t, -1, clip = "off", name = paste0("axis-r-", seq_along(axes$y$right)), z= 3)
-    }
-
     # Add strips
     switch_x <- !is.null(params$switch) && params$switch %in% c("both", "x")
     switch_y <- !is.null(params$switch) && params$switch %in% c("both", "y")

From c03156475a1f9e844ea93eabe6f6b840180f01bb Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 15:52:42 +0200
Subject: [PATCH 06/17] isolate grid strip attachment

---
 R/facet-grid-.R | 145 ++++++++++++++++++++++++++----------------------
 1 file changed, 78 insertions(+), 67 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index dfdf7b0e52..8342f753ca 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -452,101 +452,107 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     table
   },
 
-  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
-    }
+  attach_strips = function(table, layout, axis_size, params, theme) {
 
     col_vars <- unique0(layout[names(params$cols)])
     row_vars <- unique0(layout[names(params$rows)])
-    # Adding labels metadata, useful for labellers
-    attr(col_vars, "type") <- "cols"
+    attr(col_vars, "type")  <- "cols"
+    attr(row_vars, "type")  <- "rows"
     attr(col_vars, "facet") <- "grid"
-    attr(row_vars, "type") <- "rows"
     attr(row_vars, "facet") <- "grid"
-    strips <- render_strips(col_vars, row_vars, params$labeller, theme)
 
-    aspect_ratio <- theme$aspect.ratio
-    if (!is.null(aspect_ratio) && (params$space_free$x || params$space_free$y)) {
-      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
-    }
+    strips <- render_strips(col_vars, row_vars, params$labeller, theme)
 
-    panel_table <- self$init_panels(
-      panels, layout, theme, ranges, params,
-      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]])
-    )
+    switch_x <- !is.null(params$switch) && params$switch %in% c("both", "x")
+    switch_y <- !is.null(params$switch) && params$switch %in% c("both", "y")
 
-    panel_table <- self$attach_axes(
-      panel_table, layout, ranges, coord, theme, params
-    )
+    inside_x <- (calc_element("strip.placement.x", theme) %||% "inside") == "inside"
+    inside_y <- (calc_element("strip.placement.y", theme) %||% "inside") == "inside"
 
-    ncol <- max(layout$COL)
-    nrow <- max(layout$ROW)
-    mtx <- function(x) matrix(x, nrow = nrow, ncol = ncol, byrow = TRUE)
+    padding <- convertUnit(calc_element("strip.switch.pad.grid", theme), "cm")
 
-    # Add strips
-    switch_x <- !is.null(params$switch) && params$switch %in% c("both", "x")
-    switch_y <- !is.null(params$switch) && params$switch %in% c("both", "y")
-    inside_x <- (theme$strip.placement.x %||% theme$strip.placement %||% "inside") == "inside"
-    inside_y <- (theme$strip.placement.y %||% theme$strip.placement %||% "inside") == "inside"
-    strip_padding <- convertUnit(calc_element("strip.switch.pad.grid", theme), "cm")
-    panel_pos_col <- panel_cols(panel_table)
+    args <- list(l = panel_cols(table)$l, clip = "on", z = 2)
     if (switch_x) {
-      if (!is.null(strips$x$bottom)) {
-        if (inside_x) {
-          panel_table <- gtable_add_rows(panel_table, max_height(strips$x$bottom), -2)
-          panel_table <- gtable_add_grob(panel_table, strips$x$bottom, -2, panel_pos_col$l, clip = "on", name = paste0("strip-b-", seq_along(strips$x$bottom)), z = 2)
-        } else {
-          if (!all(vapply(axes$x$bottom, is.zero, logical(1)))) {
-            panel_table <- gtable_add_rows(panel_table, strip_padding, -1)
+      strip <- strips$x$bottom
+      if (!is.null(strip)) {
+        args[c("name", "t")] <- list(paste0("strip-b-", seq_along(strip)), -2)
+        if (!inside_x) {
+          if (table_has_grob(table, "axis-b")) {
+            table <- gtable_add_rows(table, padding, -1)
           }
-          panel_table <- gtable_add_rows(panel_table, max_height(strips$x$bottom), -1)
-          panel_table <- gtable_add_grob(panel_table, strips$x$bottom, -1, panel_pos_col$l, clip = "on", name = paste0("strip-b-", seq_along(strips$x$bottom)), z = 2)
+          args$t <- -1
         }
+        table <- gtable_add_rows(table, max_height(strip), args$t)
       }
     } else {
-      if (!is.null(strips$x$top)) {
-        if (inside_x) {
-          panel_table <- gtable_add_rows(panel_table, max_height(strips$x$top), 1)
-          panel_table <- gtable_add_grob(panel_table, strips$x$top, 2, panel_pos_col$l, clip = "on", name = paste0("strip-t-", seq_along(strips$x$top)), z = 2)
-        } else {
-          if (!all(vapply(axes$x$top, is.zero, logical(1)))) {
-            panel_table <- gtable_add_rows(panel_table, strip_padding, 0)
+      strip <- strips$x$top
+      if (!is.null(strip)) {
+        args[c("name", "t")] <- list(paste0("strip-t-", seq_along(strip)), 2)
+        if (!inside_x) {
+          if (table_has_grob(table, "axis-t")) {
+            table <- gtable_add_rows(table, padding, 0)
           }
-          panel_table <- gtable_add_rows(panel_table, max_height(strips$x$top), 0)
-          panel_table <- gtable_add_grob(panel_table, strips$x$top, 1, panel_pos_col$l, clip = "on", name = paste0("strip-t-", seq_along(strips$x$top)), z = 2)
+          args$t <- 1
         }
+        table <- gtable_add_rows(table, max_height(strip), args$t - 1)
       }
     }
-    panel_pos_rows <- panel_rows(panel_table)
+    if (!is.null(strip)) {
+      table <- inject(gtable_add_grob(table, strip, !!!args))
+    }
+
+    args <- list(t = panel_rows(table)$t, clip = "on", z = 2)
     if (switch_y) {
-      if (!is.null(strips$y$left)) {
-        if (inside_y) {
-          panel_table <- gtable_add_cols(panel_table, max_width(strips$y$left), 1)
-          panel_table <- gtable_add_grob(panel_table, strips$y$left, panel_pos_rows$t, 2, clip = "on", name = paste0("strip-l-", seq_along(strips$y$left)), z = 2)
-        } else {
-          if (!all(vapply(axes$y$left, is.zero, logical(1)))) {
-            panel_table <- gtable_add_cols(panel_table, strip_padding, 0)
+      strip <- strips$y$left
+      if (!is.null(strip)) {
+        args[c("name", "l")] <- list(paste0("strip-l-", seq_along(strip)), 2)
+        if (!inside_y) {
+          if (table_has_grob(table, "axis-l")) {
+            table <- gtable_add_cols(table, padding, 0)
           }
-          panel_table <- gtable_add_cols(panel_table, max_width(strips$y$left), 0)
-          panel_table <- gtable_add_grob(panel_table, strips$y$left, panel_pos_rows$t, 1, clip = "on", name = paste0("strip-l-", seq_along(strips$y$left)), z = 2)
+          args$l <- 1
         }
+        table <- gtable_add_cols(table, max_width(strip), args$l - 1)
       }
     } else {
-      if (!is.null(strips$y$right)) {
-        if (inside_y) {
-          panel_table <- gtable_add_cols(panel_table, max_width(strips$y$right), -2)
-          panel_table <- gtable_add_grob(panel_table, strips$y$right, panel_pos_rows$t, -2, clip = "on", name = paste0("strip-r-", seq_along(strips$y$right)), z = 2)
-        } else {
-          if (!all(vapply(axes$y$right, is.zero, logical(1)))) {
-            panel_table <- gtable_add_cols(panel_table, strip_padding, -1)
+      strip <- strips$y$right
+      if (!is.null(strip)) {
+        args[c("name", "l")] <- list(paste0("strip-r-", seq_along(strip)), -2)
+        if (!inside_y) {
+          if (table_has_grob(table, "axis-r")) {
+            table <- gtable_add_cols(table, padding, -1)
           }
-          panel_table <- gtable_add_cols(panel_table, max_width(strips$y$right), -1)
-          panel_table <- gtable_add_grob(panel_table, strips$y$right, panel_pos_rows$t, -1, clip = "on", name = paste0("strip-r-", seq_along(strips$y$right)), z = 2)
+          args$l <- -1
         }
+        table <- gtable_add_cols(table, max_width(strip), args$l)
       }
     }
-    panel_table
+    if (!is.null(strip)) {
+      table <- inject(gtable_add_grob(table, strip, !!!args))
+    }
+    table
+  },
+
+  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
+    if ((params$free$x || params$free$y) && !coord$is_free()) {
+      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
+    }
+
+    aspect_ratio <- theme$aspect.ratio
+    if (!is.null(aspect_ratio) && (params$space_free$x || params$space_free$y)) {
+      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
+    }
+
+    panel_table <- self$init_panels(
+      panels, layout, theme, ranges, params,
+      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]])
+    )
+
+    panel_table <- self$attach_axes(
+      panel_table, layout, ranges, coord, theme, params
+    )
+
+    self$attach_strips(panel_table, layout, params = params, theme = theme)
   },
   vars = function(self) {
     names(c(self$params$rows, self$params$cols))
@@ -563,3 +569,8 @@ ulevels <- function(x, na.last = TRUE) {
     sort(unique0(x), na.last = na.last)
   }
 }
+
+table_has_grob <- function(table, pattern) {
+  grobs <- table$grobs[grep(pattern, table$layout$name)]
+  !all(vapply(grobs, is.zero, logical(1)))
+}

From 665e009532271a63da59ff3e7a14b9a286dae2e8 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 16:33:53 +0200
Subject: [PATCH 07/17] eliminate need for strips to know about axis size

---
 R/facet-grid-.R |  6 +++---
 R/facet-wrap.R  | 25 +++++++++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 8342f753ca..75bcdb3952 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -411,7 +411,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       table <- weave_axes(
         table,
         lapply(axes$x, function(x) matrix(x[x_order], dim[1], dim[2], byrow = TRUE))
-      )$panels
+      )
     } else {
       table  <- gtable_add_rows(table, max_height(axes$x$top), 0)
       table  <- gtable_add_rows(table, max_height(axes$x$bottom), -1)
@@ -432,7 +432,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       table <- weave_axes(
         table,
         lapply(axes$y, function(y) matrix(y[y_order], dim[1], dim[2], byrow = TRUE))
-      )$panels
+      )
     } else {
       table  <- gtable_add_cols(table, max_width(axes$y$left), 0)
       table  <- gtable_add_cols(table, max_width(axes$y$right), -1)
@@ -452,7 +452,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     table
   },
 
-  attach_strips = function(table, layout, axis_size, params, theme) {
+  attach_strips = function(table, layout, params, theme) {
 
     col_vars <- unique0(layout[names(params$cols)])
     row_vars <- unique0(layout[names(params$rows)])
diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 3caff411da..734665c74d 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -301,8 +301,8 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     free        <- params$free        %||% list(x = FALSE, y = FALSE)
 
     # Render individual axes
-    ranges <- censor_labels(ranges, layout, axis_labels)
-    original   <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
+    ranges   <- censor_labels(ranges, layout, axis_labels)
+    original <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
 
     # Sort axes
     x_order <- if (axis_labels$x) layout$SCALE_X else seq_len(nrow(layout))
@@ -406,7 +406,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     weave_axes(table, axes, empty)
   },
 
-  attach_strips = function(table, layout, axis_size, params, theme) {
+  attach_strips = function(table, layout, params, theme) {
 
     # Format labels
     if (length(params$facets) == 0) {
@@ -426,8 +426,8 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     # Set position invariant parameters
     padding  <- convertUnit(calc_element("strip.switch.pad.wrap", theme), "cm")
     position <- params$strip.position %||% "top"
-    prefix   <- paste0("strip-", substr(position, 1, 1))
-    pad <- axis_size[[position]]
+    pos      <- substr(position, 1, 1)
+    prefix   <- paste0("strip-", pos)
 
     # Setup weaving table
     dim <- c(max(layout$ROW), max(layout$COL))
@@ -444,7 +444,6 @@ FacetWrap <- ggproto("FacetWrap", Facet,
       inside  <- "strip.placement.y"
       size    <- apply(mat, 2, max_width, value_only = TRUE)
       weave   <- weave_tables_col
-      pad[as.numeric(pad) != 0] <- padding
     }
 
     inside <- (calc_element(inside, theme) %||% "inside") == "inside"
@@ -453,9 +452,15 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     size   <- unit(size, "cm")
 
     table <- weave(table, mat, shift, size, name = prefix, z = 2, clip = "on")
+
     if (!inside) {
-      pad[as.numeric(pad) != 0] <- padding
-      table <- weave(table, missing_arg(), shift, pad)
+      axes  <- grepl(paste0("axis-", pos), table$layout$name)
+      has_axes <- !vapply(table$grobs[axes], is.zero, logical(1))
+      has_axes <- split(has_axes, table$layout[[pos]][axes])
+      has_axes <- vapply(has_axes, sum, numeric(1)) > 0
+      padding  <- rep(padding, length(has_axes))
+      padding[!has_axes] <- unit(0, "cm")
+      table <- weave(table, , shift, padding)
     }
 
     table
@@ -493,7 +498,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
 
     panel_table <- self$attach_axes(panel_table, layout, ranges, coord, theme, params)
 
-    self$attach_strips(panel_table$panels, layout, panel_table$sizes, params, theme)
+    self$attach_strips(panel_table, layout, params, theme)
   },
   vars = function(self) {
     names(self$params$facets)
@@ -575,7 +580,7 @@ weave_axes <- function(panels, axes, empty = NULL, z = 3L) {
   for (i in seq_along(axes)) {
     panels <- weave[[i]](panels, axes[[i]], shift[i], sizes[[i]], names[i], z = z)
   }
-  list(panels = panels, sizes = sizes)
+  panels
 }
 
 # Measures the size of axes while ignoring those bordering empty panels

From 13aebf8e12674b2b389f1dc3b30ac900e40e1187 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 29 May 2024 17:06:11 +0200
Subject: [PATCH 08/17] make formals consistent

---
 R/facet-grid-.R | 21 ++++++++++++++-------
 R/facet-wrap.R  |  6 +++---
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 75bcdb3952..4dfc7a45dc 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -336,16 +336,18 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     data
   },
 
-  init_panels = function(panels, layout, theme, ranges, params, aspect_ratio, clip = "on") {
+  init_gtable = function(panels, layout, theme, ranges, params, aspect_ratio = NULL, clip = "on") {
 
+    # Initialise matrix of panels
     dim   <- c(max(layout$ROW), max(layout$COL))
     table <- matrix(panels, dim[1], dim[2], byrow = TRUE)
 
-    space <- params$space_free %||% list(x = FALSE, y = FALSE)
-
+    # Set initial sizes
     widths  <- unit(rep(1, dim[2]), "null")
     heights <- unit(rep(1 * abs(aspect_ratio %||% 1), dim[1]), "null")
 
+    # When space are free, let panel parameter limits determine size of panel
+    space <- params$space_free %||% list(x = FALSE, y = FALSE)
     if (space$x) {
       idx    <- layout$PANEL[layout$ROW == 1]
       widths <- vapply(idx, function(i) diff(ranges[[i]]$x.range), numeric(1))
@@ -358,6 +360,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       heights <- unit(heights, "null")
     }
 
+    # Build gtable
     table <- gtable_matrix(
       "layout", table,
       widths = widths, heights = heights,
@@ -365,6 +368,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       clip = clip, z = matrix(1, dim[1], dim[2])
     )
 
+    # Set panel names
     table$layout$name <- paste(
       "panel",
       rep(seq_len(dim[2]), dim[1]),
@@ -372,6 +376,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       sep = "-"
     )
 
+    # Add spacing between panels
     spacing <- lapply(
       c(x = "panel.spacing.x", y = "panel.spacing.y"),
       calc_element, theme = theme
@@ -384,11 +389,11 @@ FacetGrid <- ggproto("FacetGrid", Facet,
 
   attach_axes = function(table, layout, ranges, coord, theme, params) {
 
-    dim <- c(max(layout$ROW), max(layout$COL))
-
+    # Setup parameters
     draw_axes   <- params$draw_axes   %||% list(x = FALSE, y = FALSE)
     axis_labels <- params$axis_labels %||% list(x = TRUE,  y = TRUE)
 
+    dim <- c(max(layout$ROW), max(layout$COL))
     if (!axis_labels$x) {
       cols    <- seq_len(nrow(layout))
       x_order <- as.integer(layout$PANEL[order(layout$ROW, layout$COL)])
@@ -404,6 +409,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       y_order <- layout$ROW
     }
 
+    # Render individual axes
     ranges <- censor_labels(ranges, layout, axis_labels)
     axes   <- render_axes(ranges[cols], ranges[rows], coord, theme, transpose = TRUE)
 
@@ -543,9 +549,10 @@ FacetGrid <- ggproto("FacetGrid", Facet,
       cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
     }
 
-    panel_table <- self$init_panels(
+    panel_table <- self$init_gtable(
       panels, layout, theme, ranges, params,
-      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]])
+      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]]),
+      clip = coord$clip
     )
 
     panel_table <- self$attach_axes(
diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 734665c74d..94ddfaf120 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -258,7 +258,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     data
   },
 
-  init_panels = function(panels, layout, theme, aspect_ratio = NULL, clip = "on") {
+  init_gtable = function(panels, layout, theme, ranges, params, aspect_ratio = NULL, clip = "on") {
 
     dim <- c(max(layout$ROW), max(layout$COL))
 
@@ -488,8 +488,8 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     layout <- layout[panel_order, ]
     panels <- panels[panel_order]
 
-    panel_table <- self$init_panels(
-      panels, layout, theme,
+    panel_table <- self$init_gtable(
+      panels, layout, theme, ranges, params,
       # If user hasn't set aspect ratio, ask the coordinate system if
       # it wants to specify one
       aspect_ratio = theme$aspect.ratio %||% coord$aspect(ranges[[1]]),

From bae5befc66d2f86d0127267f8de3438059558740 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Thu, 30 May 2024 11:27:11 +0200
Subject: [PATCH 09/17] include all cases of grid strip/axis interactions in
 tests

---
 tests/testthat/test-facet-strips.R | 37 ++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/tests/testthat/test-facet-strips.R b/tests/testthat/test-facet-strips.R
index 1ee8792e99..c426015107 100644
--- a/tests/testthat/test-facet-strips.R
+++ b/tests/testthat/test-facet-strips.R
@@ -148,25 +148,52 @@ test_that("strips can be removed", {
 
 test_that("padding is only added if axis is present", {
   p <- ggplot(data = mpg, aes(x = displ, y = hwy)) +
-    facet_grid(. ~ drv) +
+    facet_grid(year ~ drv) +
     theme(
       strip.placement = "outside",
       strip.switch.pad.grid = unit(10, "mm")
     )
   pg <- ggplotGrob(p)
-  expect_equal(length(pg$heights), 17)
+  expect_equal(length(pg$heights), 19)
+  expect_equal(length(pg$widths), 18)
 
-  pg <- ggplotGrob(p + scale_x_continuous(position = "top"))
-  expect_equal(length(pg$heights), 18)
+  pg <- ggplotGrob(
+    p + scale_x_continuous(position = "top") +
+      scale_y_continuous(position = "right")
+  )
+  expect_equal(length(pg$heights), 20)
   expect_equal(as.character(pg$heights[9]), "1cm")
+  expect_equal(length(pg$widths), 19)
+  expect_equal(as.character(pg$widths[13]), "1cm")
 
   # Also add padding with negative ticks and no text (#5251)
   pg <- ggplotGrob(
     p + scale_x_continuous(labels = NULL, position = "top") +
       theme(axis.ticks.length.x.top = unit(-2, "mm"))
   )
-  expect_equal(length(pg$heights), 18)
+  expect_equal(length(pg$heights), 20)
   expect_equal(as.character(pg$heights[9]), "1cm")
+
+  # Inverse should be true when strips are switched
+  p <- ggplot(data = mpg, aes(x = displ, y = hwy)) +
+    facet_grid(year ~ drv, switch = "both") +
+    theme(
+      strip.placement = "outside",
+      strip.switch.pad.grid = unit(10, "mm")
+    )
+
+  pg <- ggplotGrob(p)
+  expect_equal(length(pg$heights), 20)
+  expect_equal(as.character(pg$heights[13]), "1cm")
+  expect_equal(length(pg$widths), 19)
+  expect_equal(as.character(pg$widths[7]), "1cm")
+
+  pg <- ggplotGrob(
+    p + scale_x_continuous(position = "top") +
+      scale_y_continuous(position = "right")
+  )
+  expect_equal(length(pg$heights), 19)
+  expect_equal(length(pg$widths), 18)
 })
 
 test_that("y strip labels are rotated when strips are switched", {

From 25174022223bf9f0b34b44a744a87de9999e2644 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Thu, 30 May 2024 11:57:44 +0200
Subject: [PATCH 10/17] use `seam_table()` helper

---
 R/facet-grid-.R | 170 ++++++++++++++++++++++--------------------------
 1 file changed, 78 insertions(+), 92 deletions(-)

diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 4dfc7a45dc..5b8446a3ec 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -412,47 +412,20 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     # Render individual axes
     ranges <- censor_labels(ranges, layout, axis_labels)
     axes   <- render_axes(ranges[cols], ranges[rows], coord, theme, transpose = TRUE)
+    mtx    <- function(x, o) matrix(x[o], dim[1], dim[2], byrow = TRUE)
 
     if (draw_axes$x) {
-      table <- weave_axes(
-        table,
-        lapply(axes$x, function(x) matrix(x[x_order], dim[1], dim[2], byrow = TRUE))
-      )
+      table <- weave_axes(table, lapply(axes$x, mtx, o = x_order))
     } else {
-      table  <- gtable_add_rows(table, max_height(axes$x$top), 0)
-      table  <- gtable_add_rows(table, max_height(axes$x$bottom), -1)
-      panels <- panel_cols(table)
-      table  <- gtable_add_grob(
-        table, axes$x$top,
-        t = 1, l = panels$l, clip = "off", z = 3,
-        name = paste0("axis-t-", seq_along(axes$x$top))
-      )
-      table <- gtable_add_grob(
-        table, axes$x$bottom,
-        t = -1, l = panels$l, clip = "off", z = 3,
-        name = paste0("axis-b-", seq_along(axes$x$bottom))
-      )
+      table <- seam_table(table, axes$x$top,    side = "top",    name = "axis-t", z = 3)
+      table <- seam_table(table, axes$x$bottom, side = "bottom", name = "axis-b", z = 3)
     }
 
     if (draw_axes$y) {
-      table <- weave_axes(
-        table,
-        lapply(axes$y, function(y) matrix(y[y_order], dim[1], dim[2], byrow = TRUE))
-      )
+      table <- weave_axes(table, lapply(axes$y, mtx, o = y_order))
     } else {
-      table  <- gtable_add_cols(table, max_width(axes$y$left), 0)
-      table  <- gtable_add_cols(table, max_width(axes$y$right), -1)
-      panels <- panel_rows(table)
-      table  <- gtable_add_grob(
-        table, axes$y$left,
-        t = panels$t, l = 1, clip = "off", z = 3,
-        name = paste0("axis-l-", seq_along(axes$y$left))
-      )
-      table <- gtable_add_grob(
-        table, axes$y$right,
-        t = panels$t, l = -1, clip = "off", z = 3,
-        name = paste0("axis-r-", seq_along(axes$y$right))
-      )
+      table <- seam_table(table, axes$y$left,  side = "left",  name = "axis-l", z = 3)
+      table <- seam_table(table, axes$y$right, side = "right", name = "axis-r", z = 3)
     }
 
     table
@@ -467,74 +440,43 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     attr(col_vars, "facet") <- "grid"
     attr(row_vars, "facet") <- "grid"
 
-    strips <- render_strips(col_vars, row_vars, params$labeller, theme)
+    strips  <- render_strips(col_vars, row_vars, params$labeller, theme)
+    padding <- convertUnit(calc_element("strip.switch.pad.grid", theme), "cm")
 
     switch_x <- !is.null(params$switch) && params$switch %in% c("both", "x")
-    switch_y <- !is.null(params$switch) && params$switch %in% c("both", "y")
-
     inside_x <- (calc_element("strip.placement.x", theme) %||% "inside") == "inside"
-    inside_y <- (calc_element("strip.placement.y", theme) %||% "inside") == "inside"
+    shift_x  <- if (inside_x) 1 else 2
 
-    padding <- convertUnit(calc_element("strip.switch.pad.grid", theme), "cm")
-
-    args <- list(l = panel_cols(table)$l, clip = "on", z = 2)
     if (switch_x) {
-      strip <- strips$x$bottom
-      if (!is.null(strip)) {
-        args[c("name", "t")] <- list(paste0("strip-b-", seq_along(strip)), -2)
-        if (!inside_x) {
-          if (table_has_grob(table, "axis-b")) {
-            table <- gtable_add_rows(table, padding, -1)
-          }
-          args$t <- -1
-        }
-        table <- gtable_add_rows(table, max_height(strip), args$t)
-      }
+      space <- if (!inside_x & table_has_grob(table, "axis-b")) padding
+      table <- seam_table(
+        table, strips$x$bottom, side = "bottom", name = "strip-b",
+        shift = shift_x, z = 2, clip = "on", spacing = space
+      )
     } else {
-      strip <- strips$x$top
-      if (!is.null(strip)) {
-        args[c("name", "t")] <- list(paste0("strip-t-", seq_along(strip)), 2)
-        if (!inside_x) {
-          if (table_has_grob(table, "axis-t")) {
-            table <- gtable_add_rows(table, padding, 0)
-          }
-          args$t <- 1
-        }
-        table <- gtable_add_rows(table, max_height(strip), args$t - 1)
-      }
-    }
-    if (!is.null(strip)) {
-      table <- inject(gtable_add_grob(table, strip, !!!args))
+      space <- if (!inside_x & table_has_grob(table, "axis-t")) padding
+      table <- seam_table(
+        table, strips$x$top, side = "top", name = "strip-t",
+        shift = shift_x, z = 2, clip = "on", spacing = space
+      )
     }
 
-    args <- list(t = panel_rows(table)$t, clip = "on", z = 2)
+    switch_y <- !is.null(params$switch) && params$switch %in% c("both", "y")
+    inside_y <- (calc_element("strip.placement.y", theme) %||% "inside") == "inside"
+    shift_y  <- if (inside_y) 1 else 2
+
     if (switch_y) {
-      strip <- strips$y$left
-      if (!is.null(strip)) {
-        args[c("name", "l")] <- list(paste0("strip-l-", seq_along(strip)), 2)
-        if (!inside_y) {
-          if (table_has_grob(table, "axis-l")) {
-            table <- gtable_add_cols(table, padding, 0)
-          }
-          args$l <- 1
-        }
-        table <- gtable_add_cols(table, max_width(strip), args$l - 1)
-      }
+      space <- if (!inside_y & table_has_grob(table, "axis-l")) padding
+      table <- seam_table(
+        table, strips$y$left, side = "left", name = "strip-l",
+        shift = shift_y, z = 2, clip = "on", spacing = space
+      )
     } else {
-      strip <- strips$y$right
-      if (!is.null(strip)) {
-        args[c("name", "l")] <- list(paste0("strip-r-", seq_along(strip)), -2)
-        if (!inside_y) {
-          if (table_has_grob(table, "axis-r")) {
-            table <- gtable_add_cols(table, padding, -1)
-          }
-          args$l <- -1
-        }
-        table <- gtable_add_cols(table, max_width(strip), args$l)
-      }
-    }
-    if (!is.null(strip)) {
-      table <- inject(gtable_add_grob(table, strip, !!!args))
+      space <- if (!inside_y & table_has_grob(table, "axis-r")) padding
+      table <- seam_table(
+        table, strips$y$right, side = "right", name = "strip-r",
+        shift = shift_y, z = 2, clip = "on", spacing = space
+      )
     }
     table
   },
@@ -581,3 +523,47 @@ table_has_grob <- function(table, pattern) {
   grobs <- table$grobs[grep(pattern, table$layout$name)]
   !all(vapply(grobs, is.zero, logical(1)))
 }
+
+seam_table <- function(table, grobs = NULL, side, shift = 1, name, z = 1,
+                       clip = "off", spacing = NULL) {
+  if (is.null(grobs)) {
+    return(table)
+  }
+
+  panel_col <- panel_cols(table)
+  panel_row <- panel_rows(table)
+
+  row <- switch(
+    side,
+    bottom = max(panel_row$b) + shift - 1L,
+    top    = min(panel_row$t) - shift,
+    panel_row$t
+  )
+
+  col <- switch(
+    side,
+    right = max(panel_col$r) + shift - 1L,
+    left  = min(panel_col$l) - shift,
+    panel_col$l
+  )
+
+  if (!is.null(spacing)) {
+    table <- switch(
+      side,
+      bottom = , top = gtable_add_rows(table, spacing, row),
+      left = , right = gtable_add_cols(table, spacing, col)
+    )
+    row <- row + as.numeric(side == "bottom")
+    col <- col + as.numeric(side == "right")
+  }
+
+  table <- switch(
+    side,
+    bottom = , top = gtable_add_rows(table, max_height(grobs), row),
+    left = , right = gtable_add_cols(table, max_width(grobs),  col)
+  )
+  name <- paste(name, seq_along(grobs), sep = "-")
+  row  <- row + as.numeric(side %in% c("top", "bottom"))
+  col  <- col + as.numeric(side %in% c("left", "right"))
+  gtable_add_grob(table, grobs, t = row, l = col, name = name, z = z, clip = clip)
+}

From c83da224fec657d08a15aec47510cdcddc2fa36e Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Thu, 30 May 2024 13:23:41 +0200
Subject: [PATCH 11/17] fix bug

---
 R/facet-wrap.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 94ddfaf120..9b40ba1d98 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -447,7 +447,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     }
 
     inside <- (calc_element(inside, theme) %||% "inside") == "inside"
-    shift  <- switch(position, top = , left = c(-1, 2), c(0, 1))
+    shift  <- switch(position, top = , left = c(-1, -2), c(0, 1))
     shift  <- if (inside) shift[1] else shift[2]
     size   <- unit(size, "cm")
 

From 47bb580f76e1812d21ce1530f976a2c510ede0a2 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Thu, 30 May 2024 13:35:12 +0200
Subject: [PATCH 12/17] remove duplicate test from #4669

---
 tests/testthat/test-facet-strips.R | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tests/testthat/test-facet-strips.R b/tests/testthat/test-facet-strips.R
index c426015107..ece67935a4 100644
--- a/tests/testthat/test-facet-strips.R
+++ b/tests/testthat/test-facet-strips.R
@@ -135,17 +135,6 @@ test_that("strips can be removed", {
   expect_true(all(sapply(strip_grobs, inherits, 'zeroGrob')))
 })
 
-test_that("strips can be removed", {
-  dat <- data_frame(a = rep(LETTERS[1:10], 10), x = rnorm(100), y = rnorm(100))
-  g <- ggplot(dat, aes(x = x, y = y)) +
-    geom_point() +
-    facet_wrap(~a) +
-    theme(strip.background = element_blank(), strip.text = element_blank())
-  g_grobs <- ggplotGrob(g)
-  strip_grobs <- g_grobs$grobs[grepl('strip-', g_grobs$layout$name)]
-  expect_true(all(sapply(strip_grobs, inherits, 'zeroGrob')))
-})
-
 test_that("padding is only added if axis is present", {
   p <- ggplot(data = mpg, aes(x = displ, y = hwy)) +
     facet_grid(year ~ drv) +

From efbace338b27a70822652c644675d5a87abdb5da Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Tue, 4 Jun 2024 16:08:50 +0200
Subject: [PATCH 13/17] share init_gtable method

---
 R/facet-.R      | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 R/facet-grid-.R | 51 -------------------------------------------
 R/facet-wrap.R  | 35 -----------------------------
 3 files changed, 58 insertions(+), 86 deletions(-)

diff --git a/R/facet-.R b/R/facet-.R
index abdd373d05..16f9cfda07 100644
--- a/R/facet-.R
+++ b/R/facet-.R
@@ -173,6 +173,64 @@ Facet <- ggproto("Facet", NULL,
   finish_data = function(data, layout, x_scales, y_scales, params) {
     data
   },
+  init_gtable = function(panels, layout, theme, ranges, params,
+                         aspect_ratio = NULL, clip = "on") {
+
+    # Initialise matrix of panels
+    dim   <- c(max(layout$ROW), max(layout$COL))
+    table <- matrix(list(zeroGrob()), dim[1], dim[2])
+    table[cbind(layout$ROW, layout$COL)] <- panels
+
+    # Set initial sizes
+    widths  <- unit(rep(1, dim[2]), "null")
+    heights <- unit(rep(1 * abs(aspect_ratio %||% 1), dim[1]), "null")
+
+    # When space are free, let panel parameter limits determine size of panel
+    space <- params$space_free %||% list(x = FALSE, y = FALSE)
+    if (space$x) {
+      idx    <- layout$PANEL[layout$ROW == 1]
+      widths <- vapply(idx, function(i) diff(ranges[[i]]$x.range), numeric(1))
+      widths <- unit(widths, "null")
+    }
+
+    if (space$y) {
+      idx <- layout$PANEL[layout$COL == 1]
+      heights <- vapply(idx, function(i) diff(ranges[[i]]$y.range), numeric(1))
+      heights <- unit(heights, "null")
+    }
+
+    # Build gtable
+    table <- gtable_matrix(
+      "layout", table,
+      widths = widths, heights = heights,
+      respect = !is.null(aspect_ratio),
+      clip = clip, z = matrix(1, dim[1], dim[2])
+    )
+
+    # Set panel names
+    table$layout$name <- paste(
+      "panel",
+      rep(seq_len(dim[2]), dim[1]),
+      rep(seq_len(dim[1]), each = dim[2]),
+      sep = "-"
+    )
+
+    # Add spacing between panels
+    spacing <- lapply(
+      c(x = "panel.spacing.x", y = "panel.spacing.y"),
+      calc_element, theme = theme
+    )
+
+    table <- gtable_add_col_space(table, spacing$x)
+    table <- gtable_add_row_space(table, spacing$y)
+    table
+  },
+  attach_axes = function(table, layout, ranges, coord, theme, params) {
+    table
+  },
+  attach_strip = function(table, layout, params, theme) {
+    table
+  },
   vars = function() {
     character(0)
   }
diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 5b8446a3ec..27ca52e152 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -336,57 +336,6 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     data
   },
 
-  init_gtable = function(panels, layout, theme, ranges, params, aspect_ratio = NULL, clip = "on") {
-
-    # Initialise matrix of panels
-    dim   <- c(max(layout$ROW), max(layout$COL))
-    table <- matrix(panels, dim[1], dim[2], byrow = TRUE)
-
-    # Set initial sizes
-    widths  <- unit(rep(1, dim[2]), "null")
-    heights <- unit(rep(1 * abs(aspect_ratio %||% 1), dim[1]), "null")
-
-    # When space are free, let panel parameter limits determine size of panel
-    space <- params$space_free %||% list(x = FALSE, y = FALSE)
-    if (space$x) {
-      idx    <- layout$PANEL[layout$ROW == 1]
-      widths <- vapply(idx, function(i) diff(ranges[[i]]$x.range), numeric(1))
-      widths <- unit(widths, "null")
-    }
-
-    if (space$y) {
-      idx <- layout$PANEL[layout$COL == 1]
-      heights <- vapply(idx, function(i) diff(ranges[[i]]$y.range), numeric(1))
-      heights <- unit(heights, "null")
-    }
-
-    # Build gtable
-    table <- gtable_matrix(
-      "layout", table,
-      widths = widths, heights = heights,
-      respect = !is.null(aspect_ratio),
-      clip = clip, z = matrix(1, dim[1], dim[2])
-    )
-
-    # Set panel names
-    table$layout$name <- paste(
-      "panel",
-      rep(seq_len(dim[2]), dim[1]),
-      rep(seq_len(dim[1]), each = dim[2]),
-      sep = "-"
-    )
-
-    # Add spacing between panels
-    spacing <- lapply(
-      c(x = "panel.spacing.x", y = "panel.spacing.y"),
-      calc_element, theme = theme
-    )
-
-    table <- gtable_add_col_space(table, spacing$x)
-    table <- gtable_add_row_space(table, spacing$y)
-    table
-  },
-
   attach_axes = function(table, layout, ranges, coord, theme, params) {
 
     # Setup parameters
diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 9b40ba1d98..3a7101699a 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -258,41 +258,6 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     data
   },
 
-  init_gtable = function(panels, layout, theme, ranges, params, aspect_ratio = NULL, clip = "on") {
-
-    dim <- c(max(layout$ROW), max(layout$COL))
-
-    # Initialise matrix of panels
-    table <- matrix(list(zeroGrob()), dim[1], dim[2])
-    table[cbind(layout$ROW, layout$COL)] <- panels
-
-    table <- gtable_matrix(
-      "layout", table,
-      widths  = unit(rep(1, dim[2]), "null"),
-      heights = unit(rep(aspect_ratio %||% 1, dim[1]), "null"),
-      respect = !is.null(aspect_ratio),
-      clip = clip,
-      z = matrix(1, dim[1], dim[2])
-    )
-
-    # Set panel names
-    table$layout$name <- paste(
-      "panel",
-      rep(seq_len(dim[2]), dim[1]),
-      rep(seq_len(dim[1]), each = dim[2]),
-      sep = "-"
-    )
-
-    # Add panel spacing
-    spacing <- lapply(
-      c(x = "panel.spacing.x", y = "panel.spacing.y"),
-      calc_element, theme = theme
-    )
-    table <- gtable_add_col_space(table, spacing$x)
-    table <- gtable_add_row_space(table, spacing$y)
-    table
-  },
-
   attach_axes = function(table, layout, ranges, coord, theme, params) {
 
     # Setup parameters

From 07b158815b6d85cf06fa80c8275895c7f75e4348 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Wed, 5 Jun 2024 15:31:38 +0200
Subject: [PATCH 14/17] share majority of `draw_panels()`

---
 R/facet-.R                            | 30 ++++++++++++++++++++++++---
 R/facet-grid-.R                       | 22 --------------------
 R/facet-wrap.R                        | 18 ++++------------
 tests/testthat/_snaps/facet-layout.md |  2 +-
 4 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/R/facet-.R b/R/facet-.R
index 16f9cfda07..f985d84afc 100644
--- a/R/facet-.R
+++ b/R/facet-.R
@@ -133,8 +133,32 @@ Facet <- ggproto("Facet", NULL,
   draw_front = function(data, layout, x_scales, y_scales, theme, params) {
     rep(list(zeroGrob()), vec_unique_count(layout$PANEL))
   },
-  draw_panels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    cli::cli_abort("Not implemented.")
+  draw_panels = function(self, panels, layout, x_scales = NULL, y_scales = NULL,
+                         ranges, coord, data = NULL, theme, params) {
+
+    free  <- params$free       %||% list(x = FALSE, y = FALSE)
+    space <- params$space_free %||% list(x = FALSE, y = FALSE)
+
+    if ((free$x || free$y) && !coord$is_free()) {
+      cli::cli_abort(
+        "{.fn {snake_class(self)}} can't use free scales with \\
+        {.fn {snake_class(coord)}}."
+      )
+    }
+
+    aspect_ratio <- theme$aspect.ratio
+    if (!is.null(aspect_ratio) && (space$x || space$y)) {
+      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
+    }
+
+    table <- self$init_gtable(
+      panels, layout, theme, ranges, params,
+      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]]),
+      clip = coord$clip
+    )
+
+    table <- self$attach_axes(table, layout, ranges, coord, theme, params)
+    self$attach_strips(table, layout, params, theme)
   },
   draw_labels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, labels, params) {
     panel_dim <-  find_panel(panels)
@@ -228,7 +252,7 @@ Facet <- ggproto("Facet", NULL,
   attach_axes = function(table, layout, ranges, coord, theme, params) {
     table
   },
-  attach_strip = function(table, layout, params, theme) {
+  attach_strips = function(table, layout, params, theme) {
     table
   },
   vars = function() {
diff --git a/R/facet-grid-.R b/R/facet-grid-.R
index 27ca52e152..4d4f12f345 100644
--- a/R/facet-grid-.R
+++ b/R/facet-grid-.R
@@ -430,28 +430,6 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     table
   },
 
-  draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
-    }
-
-    aspect_ratio <- theme$aspect.ratio
-    if (!is.null(aspect_ratio) && (params$space_free$x || params$space_free$y)) {
-      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
-    }
-
-    panel_table <- self$init_gtable(
-      panels, layout, theme, ranges, params,
-      aspect_ratio = aspect_ratio %||% coord$aspect(ranges[[1]]),
-      clip = coord$clip
-    )
-
-    panel_table <- self$attach_axes(
-      panel_table, layout, ranges, coord, theme, params
-    )
-
-    self$attach_strips(panel_table, layout, params = params, theme = theme)
-  },
   vars = function(self) {
     names(c(self$params$rows, self$params$cols))
   }
diff --git a/R/facet-wrap.R b/R/facet-wrap.R
index 3a7101699a..04620ec75a 100644
--- a/R/facet-wrap.R
+++ b/R/facet-wrap.R
@@ -432,10 +432,6 @@ FacetWrap <- ggproto("FacetWrap", Facet,
   },
 
   draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}.")
-    }
-
     if (inherits(coord, "CoordFlip")) {
       if (params$free$x) {
         layout$SCALE_X <- seq_len(nrow(layout))
@@ -453,17 +449,11 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     layout <- layout[panel_order, ]
     panels <- panels[panel_order]
 
-    panel_table <- self$init_gtable(
-      panels, layout, theme, ranges, params,
-      # If user hasn't set aspect ratio, ask the coordinate system if
-      # it wants to specify one
-      aspect_ratio = theme$aspect.ratio %||% coord$aspect(ranges[[1]]),
-      clip = coord$clip
+    ggproto_parent(Facet, self)$draw_panels(
+      panels = panels, layout = layout,
+      ranges = ranges, coord = coord,
+      theme = theme, params = params
     )
-
-    panel_table <- self$attach_axes(panel_table, layout, ranges, coord, theme, params)
-
-    self$attach_strips(panel_table, layout, params, theme)
   },
   vars = function(self) {
     names(self$params$facets)
diff --git a/tests/testthat/_snaps/facet-layout.md b/tests/testthat/_snaps/facet-layout.md
index 03cdcbe8b3..142bde22fe 100644
--- a/tests/testthat/_snaps/facet-layout.md
+++ b/tests/testthat/_snaps/facet-layout.md
@@ -33,7 +33,7 @@
 
 # facet_grid throws errors at bad layout specs
 
-    `coord_fixed()` doesn't support free scales.
+    `facet_grid()` can't use free scales with `coord_fixed()`.
 
 ---
 

From cf53b0595a55f05975ad4d8a1687ef9f51244223 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Mon, 24 Jun 2024 09:40:36 +0200
Subject: [PATCH 15/17] add news bullet

---
 NEWS.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS.md b/NEWS.md
index de3e87cee3..541956cdad 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 # ggplot2 (development version)
 
+* (internal) rearranged the code of `Facet$draw_paensl()` method (@teunbrand).
 * The `arrow.fill` parameter is now applied to more line-based functions: 
   `geom_path()`, `geom_line()`, `geom_step()` `geom_function()`, line 
    geometries in `geom_sf()` and `element_line()`.

From 7908e224e03fce65ce910cd85f3de3b2846970d6 Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Mon, 24 Jun 2024 10:04:55 +0200
Subject: [PATCH 16/17] Update snapshots for cli 3.6.3

---
 tests/testthat/_snaps/coord-cartesian.md | 2 +-
 tests/testthat/_snaps/coord-flip.md      | 2 +-
 tests/testthat/_snaps/coord-map.md       | 2 +-
 tests/testthat/_snaps/coord-transform.md | 2 +-
 tests/testthat/_snaps/coord_sf.md        | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/testthat/_snaps/coord-cartesian.md b/tests/testthat/_snaps/coord-cartesian.md
index 7da67ba9c9..e7ed10569a 100644
--- a/tests/testthat/_snaps/coord-cartesian.md
+++ b/tests/testthat/_snaps/coord-cartesian.md
@@ -1,6 +1,6 @@
 # cartesian coords throws error when limits are badly specified
 
-    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition/ScaleContinuous/Scale/ggproto/gg> object.
+    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition> object.
 
 ---
 
diff --git a/tests/testthat/_snaps/coord-flip.md b/tests/testthat/_snaps/coord-flip.md
index b7717a7381..99806717ba 100644
--- a/tests/testthat/_snaps/coord-flip.md
+++ b/tests/testthat/_snaps/coord-flip.md
@@ -1,6 +1,6 @@
 # flip coords throws error when limits are badly specified
 
-    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition/ScaleContinuous/Scale/ggproto/gg> object.
+    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition> object.
 
 ---
 
diff --git a/tests/testthat/_snaps/coord-map.md b/tests/testthat/_snaps/coord-map.md
index 372d54df39..011a6dd41f 100644
--- a/tests/testthat/_snaps/coord-map.md
+++ b/tests/testthat/_snaps/coord-map.md
@@ -1,6 +1,6 @@
 # coord map throws error when limits are badly specified
 
-    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition/ScaleContinuous/Scale/ggproto/gg> object.
+    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition> object.
 
 ---
 
diff --git a/tests/testthat/_snaps/coord-transform.md b/tests/testthat/_snaps/coord-transform.md
index 14be4bd125..def35a0f27 100644
--- a/tests/testthat/_snaps/coord-transform.md
+++ b/tests/testthat/_snaps/coord-transform.md
@@ -1,6 +1,6 @@
 # coord_trans() throws error when limits are badly specified
 
-    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition/ScaleContinuous/Scale/ggproto/gg> object.
+    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition> object.
 
 ---
 
diff --git a/tests/testthat/_snaps/coord_sf.md b/tests/testthat/_snaps/coord_sf.md
index 486763d781..bb43424d33 100644
--- a/tests/testthat/_snaps/coord_sf.md
+++ b/tests/testthat/_snaps/coord_sf.md
@@ -21,7 +21,7 @@
 
 # coord_sf() throws error when limits are badly specified
 
-    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition/ScaleContinuous/Scale/ggproto/gg> object.
+    `xlim` must be a vector of length 2, not a <ScaleContinuousPosition> object.
 
 ---
 

From 7bf40ac513820a265de9711cac14b29be290330d Mon Sep 17 00:00:00 2001
From: Teun van den Brand <tahvdbrand@gmail.com>
Date: Mon, 24 Jun 2024 10:05:19 +0200
Subject: [PATCH 17/17] fix note

---
 R/stat-ecdf.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/stat-ecdf.R b/R/stat-ecdf.R
index fc55dc2e04..85287b9130 100644
--- a/R/stat-ecdf.R
+++ b/R/stat-ecdf.R
@@ -183,7 +183,7 @@ wecdf <- function(x, weights = NULL) {
   )
 
   # Like `ecdf(x)`, we return an approx function
-  approxfun(
+  stats::approxfun(
     vals,
     cumsum(agg_weights) / total,
     method = "constant",