From 07f3835c8c1cf2013a13ec7a89a09eb7912045b1 Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Sat, 4 Oct 2025 14:33:57 +0530 Subject: [PATCH 1/5] This fixes issue #234 where coord_equal() was producing unnecessarily small plots that didn't fill the available plotting area. --- NEWS.md | 4 +++ R/z_facets.R | 6 +++- inst/htmljs/animint.js | 13 ++++++-- .../testthat/test-compiler-coord-equal-fix.R | 32 +++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 tests/testthat/test-compiler-coord-equal-fix.R diff --git a/NEWS.md b/NEWS.md index 35c848a08..9cb2b5572 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,10 @@ - Improve common chunk detection, output `na_group` and `row_in_group` when there are missing values. +# Changes in version 2025.10.4 (Issue #234) + +- Fixed `coord_equal()` and `coord_fixed()` to properly fill available plotting space. Previously, plots with fixed aspect ratios were unnecessarily shrunk due to incorrect normalization in the `fixed_spaces()` function. The fix changes from using `min(z, 1)` to normalizing by the maximum value across both dimensions, ensuring at least one dimension fills the available space while maintaining the correct aspect ratio. + # Changes in version 2025.10.3 (PR#240) - `guide_legend(override.aes)` works in a plot with both color and fill legends. diff --git a/R/z_facets.R b/R/z_facets.R index 26c72a404..6eb2f75ac 100644 --- a/R/z_facets.R +++ b/R/z_facets.R @@ -145,5 +145,9 @@ fixed_spaces <- function(ranges, ratio = 1) { function(z) diff(z$y.range) / diff(z$x.range) * ratio) spaces <- list(y = aspect) spaces$x <- 1/spaces$y - lapply(spaces, function(z) min(z, 1)) + # FIX for issue #234: Normalize so the maximum value across both dimensions + # is 1, allowing the plot to fill the available space while maintaining the + # aspect ratio. Previously used min(z, 1) which caused excessive shrinking. + max_val <- max(unlist(spaces)) + lapply(spaces, function(z) z / max_val) } diff --git a/inst/htmljs/animint.js b/inst/htmljs/animint.js index 02c8d760a..ea6445c88 100644 --- a/inst/htmljs/animint.js +++ b/inst/htmljs/animint.js @@ -432,12 +432,21 @@ var animint = function (to_select, json_file) { } else { var aspect = 1; } + // FIX for issue #234: Remove Math.min() that was shrinking proportions. + // The R side already normalized properly, so we just apply the aspect ratio. var wp = p_info.layout.width_proportion.map(function(x){ - return x * Math.min(1, aspect); + return x * aspect; }) var hp = p_info.layout.height_proportion.map(function(x){ - return x * Math.min(1, 1/aspect); + return x * (1/aspect); }) + // Normalize so at least one dimension fills the space + var all_props = wp.concat(hp); + var max_prop = Math.max.apply(null, all_props); + if (max_prop > 0) { + wp = wp.map(function(x) { return x / max_prop; }); + hp = hp.map(function(x) { return x / max_prop; }); + } // track the proportion of the graph that should be 'blank' // this is mainly used to implement coord_fixed() diff --git a/tests/testthat/test-compiler-coord-equal-fix.R b/tests/testthat/test-compiler-coord-equal-fix.R new file mode 100644 index 000000000..347dcbdbd --- /dev/null +++ b/tests/testthat/test-compiler-coord-equal-fix.R @@ -0,0 +1,32 @@ +acontext("coord_equal fix for issue #234") + +test_that("fixed_spaces normalizes to fill available space", { + # Test with iris petal data ranges (issue #234 example) + ranges <- list(list(x.range = c(1, 7), y.range = c(0, 2.5))) + spaces <- animint2:::fixed_spaces(ranges, ratio = 1) + + # At least one dimension should be 1 (fills available space) + max_prop <- max(spaces$x, spaces$y) + expect_equal(max_prop, 1) + + # Both proportions should be positive and <= 1 + expect_true(all(spaces$x > 0 & spaces$x <= 1)) + expect_true(all(spaces$y > 0 & spaces$y <= 1)) +}) + +test_that("coord_equal compiles and creates proper layout", { + library(data.table) + data.mat <- as.matrix(iris[,c("Petal.Width","Petal.Length")]) + + # Create the visualization from issue #234 + viz <- animint( + plot1=ggplot(data=data.table(data.mat), aes(Petal.Length, Petal.Width))+ + geom_point()+ + coord_equal()+ + theme_animint(width=800) + ) + + # Verify it compiles without error + info <- animint2dir(viz, open.browser = FALSE) + expect_true(file.exists(file.path(info$out.dir, "plot.json"))) +}) \ No newline at end of file From ab19433cb7d57a8a42beb741b8d796fb3202011c Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Mon, 6 Oct 2025 22:01:33 +0530 Subject: [PATCH 2/5] Improve tests for coord_equal fix (issue #234) --- .../testthat/test-compiler-coord-equal-fix.R | 23 ++----- tests/testthat/test-renderer-coord_equal.R | 62 +++++++++++++++++++ 2 files changed, 66 insertions(+), 19 deletions(-) create mode 100644 tests/testthat/test-renderer-coord_equal.R diff --git a/tests/testthat/test-compiler-coord-equal-fix.R b/tests/testthat/test-compiler-coord-equal-fix.R index 347dcbdbd..5e191eb3b 100644 --- a/tests/testthat/test-compiler-coord-equal-fix.R +++ b/tests/testthat/test-compiler-coord-equal-fix.R @@ -10,23 +10,8 @@ test_that("fixed_spaces normalizes to fill available space", { expect_equal(max_prop, 1) # Both proportions should be positive and <= 1 - expect_true(all(spaces$x > 0 & spaces$x <= 1)) - expect_true(all(spaces$y > 0 & spaces$y <= 1)) + expect_gt(spaces$x, 0) + expect_lte(spaces$x, 1) + expect_gt(spaces$y, 0) + expect_lte(spaces$y, 1) }) - -test_that("coord_equal compiles and creates proper layout", { - library(data.table) - data.mat <- as.matrix(iris[,c("Petal.Width","Petal.Length")]) - - # Create the visualization from issue #234 - viz <- animint( - plot1=ggplot(data=data.table(data.mat), aes(Petal.Length, Petal.Width))+ - geom_point()+ - coord_equal()+ - theme_animint(width=800) - ) - - # Verify it compiles without error - info <- animint2dir(viz, open.browser = FALSE) - expect_true(file.exists(file.path(info$out.dir, "plot.json"))) -}) \ No newline at end of file diff --git a/tests/testthat/test-renderer-coord_equal.R b/tests/testthat/test-renderer-coord_equal.R new file mode 100644 index 000000000..4aac94a65 --- /dev/null +++ b/tests/testthat/test-renderer-coord_equal.R @@ -0,0 +1,62 @@ +acontext("coord_equal renderer tests") + +test_that("coord_equal fills available space (issue #234)", { + # Create the iris petal plot from issue #234 + # This test ensures coord_equal() doesn't shrink the plot unnecessarily + library(data.table) + data.mat <- as.matrix(iris[,c("Petal.Width","Petal.Length")]) + + # Create the visualization with a specific size + viz <- animint( + plot1=ggplot(data=data.table(data.mat), aes(Petal.Length, Petal.Width))+ + geom_point()+ + coord_equal()+ + theme_animint(width=500, height=500) + ) + + # Render the plot using animint2HTML (not animint2dir) + info <- animint2HTML(viz) + + # Extract SVG plotting area dimensions + # Get the SVG element to check overall plot dimensions + svg.nodes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]') + expect_equal(length(svg.nodes), 1) + svg.attrs <- xmlAttrs(svg.nodes[[1]]) + + # Extract width and height attributes + svg.width <- as.numeric(svg.attrs[["width"]]) + svg.height <- as.numeric(svg.attrs[["height"]]) + + # The key test: with coord_equal(), at least one dimension should + # fill the available space (close to 500px as specified in theme_animint) + # On the old buggy code (before the fix in commit e4b9634b), BOTH dimensions + # would be unnecessarily shrunk below the available space. + # With the fix, at least one dimension uses nearly all available space. + + # The iris petal data has aspect ratio of ~0.417 (y_range=2.5, x_range=6) + # So with coord_equal (ratio=1), the x-axis fills space (width≈500) + # and y-axis is constrained by aspect ratio (height≈200) + + # Test that at least ONE dimension is close to requested size (500) + max_dimension <- max(svg.width, svg.height) + expect_gt(max_dimension, 450, + info="At least one dimension should fill available space") + + # Both dimensions should be positive + expect_gt(svg.width, 0) + expect_gt(svg.height, 0) + + # Verify aspect ratio is maintained (ratio = 1 for coord_equal) + x.axes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]//g[contains(@class, "xaxis")]') + y.axes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]//g[contains(@class, "yaxis")]') + + expect_equal(length(x.axes), 1) + expect_equal(length(y.axes), 1) + + xdiff <- getTickDiff(x.axes[[1]]) + ydiff <- getTickDiff(y.axes[[1]], axis = "y") + + # With ratio = 1 (coord_equal), normalized diffs should be equal + diffs <- normDiffs(xdiff, ydiff, ratio = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) +}) From 7b53778e9f40d7a88e248d98d2fdbe8eceb99b5e Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Wed, 15 Oct 2025 11:14:00 -0400 Subject: [PATCH 3/5] some test code --- tests/testthat/test-renderer1-coord.R | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-renderer1-coord.R b/tests/testthat/test-renderer1-coord.R index a4e8521f4..7efe099a3 100644 --- a/tests/testthat/test-renderer1-coord.R +++ b/tests/testthat/test-renderer1-coord.R @@ -30,7 +30,27 @@ test_that("coord_fixed with shrinking y-axis", { xdiff <- getTickDiff(x.axes[[1]]) ydiff <- getTickDiff(y.axes[[1]], axis = "y") diffs <- normDiffs(xdiff, ydiff, ratio5) - expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1e-3) +}) + +test_that("coord_equal", { + info_default <- animint2HTML(animint(plot = p + coord_equal())) + bbox_default <- get_element_bbox("g.xaxis") + svg_default <- getNodeSet(info_default$html, "//svg[@id='plot_plot']")[[1]] + width_default <- xmlAttrs(svg_default)[["width"]] + info_big <- animint2HTML(animint( + plot = p + + coord_equal()+ + theme_animint(width=1000))) + bbox_big <- get_element_bbox("g.xaxis") + svg_big <- getNodeSet(info_big$html, "//svg[@id='plot_plot']")[[1]] + width_big <- xmlAttrs(svg_big)[["width"]] + x.axes <- getNodeSet(info$html, "//g[contains(@class, 'xaxis')]") + y.axes <- getNodeSet(info$html, "//g[contains(@class, 'yaxis')]") + xdiff <- getTickDiff(x.axes[[1]]) + ydiff <- getTickDiff(y.axes[[1]], axis = "y") + diffs <- normDiffs(xdiff, ydiff) + expect_equal(diffs[1], diffs[2], tolerance = 1e-3) }) test_that("coord_fixed with shrinking x-axis", { @@ -42,5 +62,5 @@ test_that("coord_fixed with shrinking x-axis", { xdiff <- getTickDiff(x.axes[[1]]) ydiff <- getTickDiff(y.axes[[1]], axis = "y") diffs <- normDiffs(xdiff, ydiff, ratio10) - expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1e-3) }) From 208161cca5fbf4c5fc29fd0a882fad581662f071 Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Wed, 15 Oct 2025 11:18:42 -0400 Subject: [PATCH 4/5] test ratio greater than 2 --- tests/testthat/test-renderer1-coord.R | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/tests/testthat/test-renderer1-coord.R b/tests/testthat/test-renderer1-coord.R index 7efe099a3..07f33a399 100644 --- a/tests/testthat/test-renderer1-coord.R +++ b/tests/testthat/test-renderer1-coord.R @@ -33,24 +33,14 @@ test_that("coord_fixed with shrinking y-axis", { expect_equal(diffs[1], diffs[2], tolerance = 1e-3) }) -test_that("coord_equal", { - info_default <- animint2HTML(animint(plot = p + coord_equal())) +test_that("xaxis width increases with coord_equal", { + p_equal <- p + coord_equal() + animint2HTML(animint(p_equal)) bbox_default <- get_element_bbox("g.xaxis") - svg_default <- getNodeSet(info_default$html, "//svg[@id='plot_plot']")[[1]] - width_default <- xmlAttrs(svg_default)[["width"]] - info_big <- animint2HTML(animint( - plot = p + - coord_equal()+ - theme_animint(width=1000))) + animint2HTML(animint(p_equal+theme_animint(width=1000))) bbox_big <- get_element_bbox("g.xaxis") - svg_big <- getNodeSet(info_big$html, "//svg[@id='plot_plot']")[[1]] - width_big <- xmlAttrs(svg_big)[["width"]] - x.axes <- getNodeSet(info$html, "//g[contains(@class, 'xaxis')]") - y.axes <- getNodeSet(info$html, "//g[contains(@class, 'yaxis')]") - xdiff <- getTickDiff(x.axes[[1]]) - ydiff <- getTickDiff(y.axes[[1]], axis = "y") - diffs <- normDiffs(xdiff, ydiff) - expect_equal(diffs[1], diffs[2], tolerance = 1e-3) + ratio <- bbox_big$width/bbox_default$width + expect_gt(ratio, 2) }) test_that("coord_fixed with shrinking x-axis", { From b29d411f0274c06384cfcbb83ea08a9c5da3452a Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Wed, 15 Oct 2025 11:27:32 -0400 Subject: [PATCH 5/5] rm info --- tests/testthat/test-renderer-coord_equal.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/testthat/test-renderer-coord_equal.R b/tests/testthat/test-renderer-coord_equal.R index 4aac94a65..4c76d6c1e 100644 --- a/tests/testthat/test-renderer-coord_equal.R +++ b/tests/testthat/test-renderer-coord_equal.R @@ -39,8 +39,7 @@ test_that("coord_equal fills available space (issue #234)", { # Test that at least ONE dimension is close to requested size (500) max_dimension <- max(svg.width, svg.height) - expect_gt(max_dimension, 450, - info="At least one dimension should fill available space") + expect_gt(max_dimension, 450) # Both dimensions should be positive expect_gt(svg.width, 0)