Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ggsave(): Optionally switch between ragg and grDevices #4501

Closed
hsbadr opened this issue Jun 4, 2021 · 23 comments
Closed

ggsave(): Optionally switch between ragg and grDevices #4501

hsbadr opened this issue Jun 4, 2021 · 23 comments

Comments

@hsbadr
Copy link

hsbadr commented Jun 4, 2021

Currently, ragg doesn't catch segmentation faults due to invalid res, width, height, units, ... etc. (see r-lib/ragg#82). So, would it make sense to add sanity checks before calling the device or give an option to switch between ragg and grDevices by default (without specifying the device argument), even if ragg is available?

ggplot2/R/save.r

Lines 180 to 188 in 6d94f0d

if (requireNamespace('ragg', quietly = TRUE)) {
png_dev <- ragg::agg_png
jpeg_dev <- ragg::agg_jpeg
tiff_dev <- ragg::agg_tiff
} else {
png_dev <- grDevices::png
jpeg_dev <- grDevices::jpeg
tiff_dev <- grDevices::tiff
}

@yutannihilation
Copy link
Member

If you are complaining about the compatibility with ragg, it's already fixed. Did you try the dev version?

#4388

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

Did you try the dev version?

Yes. I'm not complaining; I'm just reporting that ragg causes segmentation fault with some devices (e.g., png) if any of res, width, height, or units is invalid. So, I'm requesting to add sanity checks before calling the device or give an option to disable ragg even if it's installed.

ggplot2/R/save.r

Lines 180 to 188 in 6d94f0d

if (requireNamespace('ragg', quietly = TRUE)) {
png_dev <- ragg::agg_png
jpeg_dev <- ragg::agg_jpeg
tiff_dev <- ragg::agg_tiff
} else {
png_dev <- grDevices::png
jpeg_dev <- grDevices::jpeg
tiff_dev <- grDevices::tiff
}

@clauswilke
Copy link
Member

Please provide a reproducible example that triggers a segmentation fault.

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

@clauswilke I'm getting segmentation fault with the development version of ggplot when calling the png device, if ragg is installed. I'm on R-devel.

Here's the simple ggplot example:

> library(ggplot2)
> p <- ggplot(mtcars, aes(mpg, wt)) + geom_point()
> ggsave("mtcars.png", plot = p)
Saving 7 x 7 in image

 *** caught segfault ***
address 0x38, cause 'memory not mapped'
Traceback:
 1: grid.Call(C_stringMetric, as.graphicsAnnot(x$label))
 2: descentDetails.text(list(label = "gjpqyQ", x = 0.5, y = 0.5,     just = "centre", hjust = NULL, vjust = NULL, rot = 0, check.overlap = FALSE,     name = "GRID.text.19", gp = list(fontsize = 8.8, fontfamily = "",         fontface = NULL), vp = NULL))
 3: (function (x) {    UseMethod("descentDetails", x)})(list(label = "gjpqyQ", x = 0.5, y = 0.5, just = "centre",     hjust = NULL, vjust = NULL, rot = 0, check.overlap = FALSE,     name = "GRID.text.19", gp = list(fontsize = 8.8, fontfamily = "",         fontface = NULL), vp = NULL))
 4: grid.Call(C_convert, x, as.integer(whatfrom), as.integer(whatto),     valid.units(unitTo))
 5: convertUnit(x, unitTo, "y", "dimension", "y", "dimension", valueOnly = valueOnly)
 6: convertHeight(grobDescent(textGrob(label = "gjpqyQ", gp = gpar(fontsize = size,     cex = cex, fontfamily = family, fontface = face))), "inches")
 7: font_descent(gp$fontfamily, gp$fontface, gp$fontsize, gp$cex)
 8: title_spec(label, x = x, y = y, hjust = hjust, vjust = vjust,     angle = angle, gp = gp, debug = debug, check.overlap = check.overlap)
 9: titleGrob(label, x, y, hjust = hj, vjust = vj, angle = angle,     gp = modify_list(element_gp, gp), margin = margin, margin_x = margin_x,     margin_y = margin_y, debug = element$debug, ...)
10: element_grob.element_text(list(family = "", face = "plain", colour = "grey30",     size = 8.8, hjust = 0.5, vjust = 1, angle = 0, lineheight = 0.9,     margin = c(2.2, 0, 0, 0), debug = FALSE, inherit.blank = TRUE),     x = c(0.0299806576402321, 0.223404255319149, 0.416827852998066,     0.610251450676983, 0.8036750483559, 0.997098646034816), margin_y = TRUE,     label = c("10", "15", "20", "25", "30", "35"), check.overlap = FALSE)
11: (function (element, ...) {    UseMethod("element_grob")})(list(family = "", face = "plain", colour = "grey30", size = 8.8,     hjust = 0.5, vjust = 1, angle = 0, lineheight = 0.9, margin = c(2.2,     0, 0, 0), debug = FALSE, inherit.blank = TRUE), x = c(0.0299806576402321, 0.223404255319149, 0.416827852998066, 0.610251450676983, 0.8036750483559, 0.997098646034816), margin_y = TRUE, label = c("10", "15", "20", "25", "30", "35"), check.overlap = FALSE)
12: exec(element_grob, label_element, `:=`(!!position_dim, break_positions),     `:=`(!!label_margin_name, TRUE), label = break_labels, check.overlap = check.overlap)
13: draw_axis_labels(break_positions = break_positions[indices],     break_labels = break_labels[indices], label_element = label_element,     is_vertical = is_vertical, check.overlap = check.overlap)
14: FUN(X[[i]], ...)
15: lapply(dodge_indices, function(indices) {    draw_axis_labels(break_positions = break_positions[indices],         break_labels = break_labels[indices], label_element = label_element,         is_vertical = is_vertical, check.overlap = check.overlap)})
16: draw_axis(break_positions = guide$key[[aesthetic]], break_labels = guide$key$.label,     axis_position = guide$position, theme = theme, check.overlap = guide$check.overlap,     angle = guide$angle, n.dodge = guide$n.dodge)
17: guide_gengrob.axis(guide, theme)
18: guide_gengrob(guide, theme)
19: panel_guides_grob(panel_params$guides, position = "bottom", theme = theme)
20: f(...)
21: coord$render_axis_h(range, theme)
22: f(...)
23: self$facet$draw_panels(panels, self$layout, self$panel_scales_x,     self$panel_scales_y, self$panel_params, self$coord, data,     theme, self$facet_params)
24: f(..., self = self)
25: layout$render(geom_grobs, data, theme, plot$labels)
26: ggplot_gtable.ggplot_built(data)
27: ggplot_gtable(data)
28: print.ggplot(x)
29: print(x)
30: grid.draw.ggplot(plot)
31: grid.draw(plot)
32: ggsave("mtcars.png", plot = p)

Possible actions:
1: abort (with core dump, if enabled)
2: normal R exit
3: exit R without saving workspace
4: exit R saving workspace

Uninstalling ragg fixes the problem because it switches to grDevices.

ggplot2/R/save.r

Lines 180 to 188 in 6d94f0d

if (requireNamespace('ragg', quietly = TRUE)) {
png_dev <- ragg::agg_png
jpeg_dev <- ragg::agg_jpeg
tiff_dev <- ragg::agg_tiff
} else {
png_dev <- grDevices::png
jpeg_dev <- grDevices::jpeg
tiff_dev <- grDevices::tiff
}

Can you make the switch between ragg and grDevices optional? i.e., replacing if (requireNamespace('ragg', quietly = TRUE)).

@yutannihilation
Copy link
Member

So, I'm requesting to add sanity checks before calling the device or give an option to disable ragg even if it's installed.

Ah, OK. Sorry, I failed to see the context.

But I cannot reproduce your issue on R 4.1.0. Maybe it's somehow related to your installation of ragg package?

library(ggplot2)
p <- ggplot(mtcars, aes(mpg, wt)) + geom_point()
ggsave("mtcars.png", plot = p)
#> Saving 7 x 5 in image

Created on 2021-06-05 by the reprex package (v2.0.0)

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

Maybe it's somehow related to your installation of ragg package?

Yes, I think it's related to the system libraries or the R options. I'm using Cairo with x11 devices on an HPC server. Disabling my R profile fixes the problem too. But, still, it'd be useful to be able to opt in/out the ragg default, regardless if it's installed.

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

@clauswilke @yutannihilation Here's exactly why it fails with my R profile. I spoke too soon; I was testing on my local machine. The problem persists on the server (on R-devel). I can fix it by explicitly setting the device argument though.

@hsbadr hsbadr changed the title ggsave(): Invalid device resolution or units (segmentation fault) ggsave(): Optionally switch between ragg and grDevices Jun 4, 2021
@clauswilke
Copy link
Member

I think we still need somebody else to reproduce this problem and/or have a concrete explanation of what is going wrong. The bug report started out with talking about invalid parameter inputs for the ragg devices, but I don't see how use of ggsave() with default parameters could lead to invalid parameter inputs.

I see two possibilities here:

  1. It's something else that is going wrong
  2. There is indeed some obscure situation under which ggsave() produces invalid parameter inputs. But it most likely depends on some other factor that we haven't identified yet. For example, does this only happen when using R devel?

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

@clauswilke Thanks you! With running into the same issue with simple cases, I think it's an ragg issue on some systems or R-devel (possibility # 1). Would it be possible to let the users decide if they want to use ragg by default? or maybe defaulting to grDevices while fully supporting the ragg devices?

That'd mean changing:

ggplot2/R/save.r

Lines 180 to 188 in 6d94f0d

if (requireNamespace('ragg', quietly = TRUE)) {
png_dev <- ragg::agg_png
jpeg_dev <- ragg::agg_jpeg
tiff_dev <- ragg::agg_tiff
} else {
png_dev <- grDevices::png
jpeg_dev <- grDevices::jpeg
tiff_dev <- grDevices::tiff
}

to:

  png_dev <- grDevices::png
  jpeg_dev <- grDevices::jpeg
  tiff_dev <- grDevices::tiff

Then, ragg devices can be used with the device argument. Alternatively, a global option to prefer the default would do it.

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

does this only happen when using R devel?

Yes, but I think it could be a combination of system libs (e.g., libpng) too since ragg passes the R-CMD-check on R-devel.

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

have a concrete explanation of what is going wrong.

I'll let you know when I find the culprit.

@hsbadr
Copy link
Author

hsbadr commented Jun 4, 2021

@clauswilke FYI. The common thing between my environment and #4499 is building R with Intel compilers and MKL.

@yutannihilation
Copy link
Member

Just curious. It seems ragg doesn't work at all on your environment. Why do you still want to keep it installed?

@hsbadr
Copy link
Author

hsbadr commented Jun 7, 2021

Just curious. It seems ragg doesn't work at all on your environment. Why do you still want to keep it installed?

Good question. Other packages import/suggest it so it gets installed.

You're right; the problem here is ragg on my system, but this issue requests that at least it isn't used by default with ggplot2::ggsave(). I can specify the device to fix the problem, but that isn't an option when other packages call it.

@yutannihilation
Copy link
Member

Other packages import/suggest it so it gets installed.

Hmm, thanks. I'm not sure providing an option is the right way to fix this problem, but now I get your point.

@clauswilke
Copy link
Member

We're using ragg by default now when available because other graphics devices have problems and generate a never-ending stream of bug reports, in particular for Windows users. If ragg doesn't compile/run on some platforms, then it's a ragg bug that needs to be fixed there. Also, if ragg doesn't work at all on your platform, then uninstalling it seems an appropriate action.

The only widely used package that imports (rather than suggests) ragg is pkgdown. So it might be worthwhile to file an issue there requesting they make it an optional dependency.

@hsbadr
Copy link
Author

hsbadr commented Jun 7, 2021

I see. Sounds good. Yes, it's a ragg issue, which is, I think, related to the system libs or permissions when accessing the graphics device. I didn't have enough time to debug it.

It's ok to close this issue if you wish. But, I'd prefer to check if ragg is working not just available before using it.

@hsbadr
Copy link
Author

hsbadr commented Jun 7, 2021

other graphics devices have problems and generate a never-ending stream of bug reports, in particular for Windows users.

Maybe, use ragg if requireNamespace('ragg', quietly = TRUE) & Sys.info()["sysname"] == "Windows"? This would address the Windows-related issues and default to grDevices for the other systems.

@thomasp85
Copy link
Member

No, ragg is the default if installed. While I can emphasise with your situation we will not roll back to a worse graphic device to address an issue of a single user when you have the option of passing in png as the device argument.

I'll be happy to investigate why ragg is failing on your system if you can somehow replicate the issue in a docker container or in some other way that makes it debuggable

@thomasp85
Copy link
Member

If you do find a way to reproduce the issue outside of your system please open an issue in the ragg repo and we can take it from there

@hsbadr
Copy link
Author

hsbadr commented Jun 9, 2021

@clauswilke @yutannihilation @thomasp85 FYI. I've been able to fix the segfault with ragg by cleanly building and relinking all packages, starting with a clean R-devel installation (empty $R_HOME/library directory). While I do upgrade all packages after building a new revision of R source, it seems that the version check (e.g., by CheckBuilt = TRUE) is not enough with a moving target (R-devel) that can introduce breaking changes at any time. Also, I think R doesn't properly and recursively relink library dependencies. So, if a package A links to B that links to C, R may need to relink A and B if C is upgraded.

@wallingTACC That might address #4499 too.

@mschnetzer
Copy link

Hi, please excuse that I comment on a closed issue. As @thomasp85 asked @hsbadr to reproduce the issue outside his system: I have similar problems with ggsave() and ragg. When I plot a figure with a custom font, ggsave() won't export the font into a PNG until i uninstall ragg. (When I manually export a PNG from the RStudio "Plots" window, it works, however). I just wanted to let you know that @hsbadr is not the only one with problems, but I am much less experienced with the technical backgrounds. Thank you!

@tylerlittlefield
Copy link

@mschnetzer Same issue on my end as well. I can't provide anything super meaningful but if anyone is noticing that rstudio plot viewer is showing fonts but ggsave() isn't, I fixed it by providing device = grDevices::png in the ggsave() call:

  ggplot2::ggsave(
    filename = filename,
    plot = p,
    width = 1920,
    height = 1080,
    units = "px",
    dpi = 200, 
    device = grDevices::png
  )

In case it helps, my system information below:

Session info ──────────────────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 4.1.1 (2021-08-10)
 os       macOS Big Sur 11.5.2        
 system   x86_64, darwin17.0          
 ui       RStudio                     
 language (EN)                        
 collate  en_US.UTF-8                 
 ctype    en_US.UTF-8                 
 tz       America/Los_Angeles         
 date     2021-09-01Packages ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 package       * version date       lib source            
 assertthat      0.2.1   2019-03-21 [1] standard (@0.2.1) 
 audio           0.1-7   2020-03-09 [1] standard (@0.1-7) 
 aws.s3          0.3.21  2020-04-07 [1] standard (@0.3.21)
 aws.signature   0.6.0   2020-06-01 [1] standard (@0.6.0) 
 base64enc       0.1-3   2015-07-28 [1] standard (@0.1-3) 
 beepr           1.3     2018-06-04 [1] standard (@1.3)   
 cli             3.0.1   2021-07-17 [1] standard (@3.0.1) 
 colorspace      2.0-2   2021-06-24 [1] standard (@2.0-2) 
 crayon          1.4.1   2021-02-08 [1] standard (@1.4.1) 
 curl            4.3.2   2021-06-23 [1] standard (@4.3.2) 
 data.table      1.14.0  2021-02-21 [1] standard (@1.14.0)
 DBI             1.1.1   2021-01-15 [1] standard (@1.1.1) 
 digest          0.6.27  2020-10-24 [1] standard (@0.6.27)
 dplyr         * 1.0.7   2021-06-18 [1] standard (@1.0.7) 
 ellipsis        0.3.2   2021-04-29 [1] standard (@0.3.2) 
 evaluate        0.14    2019-05-28 [1] standard (@0.14)  
 fansi           0.5.0   2021-05-25 [1] standard (@0.5.0) 
 farver          2.1.0   2021-02-28 [1] standard (@2.1.0) 
 fastmap         1.1.0   2021-01-25 [1] standard (@1.1.0) 
 flextable       0.6.7   2021-07-22 [1] standard (@0.6.7) 
 gdtools         0.2.3   2021-01-06 [1] standard (@0.2.3) 
 generics        0.1.0   2020-10-31 [1] standard (@0.1.0) 
 ggplot2         3.3.5   2021-06-25 [1] standard (@3.3.5) 
 glue            1.4.2   2020-08-27 [1] standard (@1.4.2) 
 gtable          0.3.0   2019-03-25 [1] standard (@0.3.0) 
 htmltools       0.5.2   2021-08-25 [1] standard (@0.5.2) 
 htmlwidgets     1.5.3   2020-12-10 [1] standard (@1.5.3) 
 httr            1.4.2   2020-07-20 [1] standard (@1.4.2) 
 knitr           1.33    2021-04-24 [1] standard (@1.33)  
 lattice         0.20-44 2021-05-02 [2] CRAN (R 4.1.1)    
 lifecycle       1.0.0   2021-02-15 [1] standard (@1.0.0) 
 lubridate       1.7.10  2021-02-26 [1] standard (@1.7.10)
 magrittr        2.0.1   2020-11-17 [1] standard (@2.0.1) 
 mdout           0.2.12  2021-08-31 [1] local             
 mdtrend         2.1.0   2021-07-07 [1] standard (@2.1.0) 
 munsell         0.5.0   2018-06-12 [1] standard (@0.5.0) 
 officer         0.3.19  2021-07-21 [1] standard (@0.3.19)
 openxlsx        4.2.4   2021-06-16 [1] standard (@4.2.4) 
 pillar          1.6.2   2021-07-29 [1] standard (@1.6.2) 
 pkgconfig       2.0.3   2019-09-22 [1] standard (@2.0.3) 
 purrr           0.3.4   2020-04-17 [1] standard (@0.3.4) 
 R6              2.5.1   2021-08-19 [1] standard (@2.5.1) 
 ragg            1.1.3   2021-06-09 [1] standard (@1.1.3) 
 Rcpp            1.0.7   2021-07-07 [1] standard (@1.0.7) 
 reactable       0.2.3   2020-10-04 [1] standard (@0.2.3) 
 rlang           0.4.11  2021-04-30 [1] standard (@0.4.11)
 rmarkdown       2.10    2021-08-06 [1] standard (@2.10)  
 rstudioapi      0.13    2020-11-12 [1] standard (@0.13)  
 rvg             0.2.5   2020-06-30 [1] standard (@0.2.5) 
 scales          1.1.1   2020-05-11 [1] standard (@1.1.1) 
 sessioninfo     1.1.1   2018-11-05 [1] standard (@1.1.1) 
 stringi         1.7.4   2021-08-25 [1] standard (@1.7.4) 
 stringr         1.4.0   2019-02-10 [1] standard (@1.4.0) 
 systemfonts     1.0.2   2021-05-11 [1] standard (@1.0.2) 
 textshaping     0.3.5   2021-06-09 [1] standard (@0.3.5) 
 tibble          3.1.4   2021-08-25 [1] standard (@3.1.4) 
 tidyr           1.1.3   2021-03-03 [1] standard (@1.1.3) 
 tidyselect      1.1.1   2021-04-30 [1] standard (@1.1.1) 
 utf8            1.2.2   2021-07-24 [1] standard (@1.2.2) 
 uuid            0.1-4   2020-02-26 [1] standard (@0.1-4) 
 vctrs           0.3.8   2021-04-29 [1] standard (@0.3.8) 
 withr           2.4.2   2021-04-18 [1] standard (@2.4.2) 
 xfun            0.25    2021-08-06 [1] standard (@0.25)  
 xml2            1.3.2   2020-04-23 [1] standard (@1.3.2) 
 zip             2.2.0   2021-05-31 [1] standard (@2.2.0) 
 zoo             1.8-9   2021-03-09 [1] standard (@1.8-9) 

[1] /Users/tlittlef/Library/R/x86_64/4.1/library
[2] /Library/Frameworks/R.framework/Versions/4.1/Resources/library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants