Record example plots with 'png()' if requested #2314

trevorld · 2023-06-01T17:20:38Z

If grDevices::png() is requested in _pkgdown.yml use it to record example plots instead of ragg::agg_png().
In particular examples with if blocks using grDevices::dev.capabilities() to ensure that a required graphics feature is supported by the active graphics device will now work correctly.
This fixes Record example plots with device requested in figures field instead of ragg::agg_png() #2299. Note here is a list of graphics features which grDevices::dev.capabilities() affirms support for grDevices::png(type = "cairo") in R 4.3 that is not affirmed in the latest version of ragg::agg_png() because it doesn't support that feature yet:
- Luminance masks
- Compositing operators
- Affine transformations
- Stroking and filling paths
- Glyphs

* If `grDevices::png()` is requested in `_pkgdown.yml` use it to record example plots instead of `ragg::agg_png()`. * In particular examples with if blocks using `grDevices::dev.capabilities()` to ensure that a required graphics feature is supported by the active graphics device will now work correctly. fixes #2299

hadley

Thanks for starting to think about this.

As well as the comment about the code, does this need changes to the documentation or tests?

hadley · 2023-10-17T17:09:31Z

R/highlight.R

@@ -14,8 +14,14 @@ highlight_examples <- function(code, topic, env = globalenv()) {

  # some options from testthat::local_reproducible_output()
  # https://github.com/r-lib/testthat/blob/47935141d430e002070a95dd8af6dbf70def0994/R/local.R#L86
+  if (any(fig_settings()$dev %in% c("png", "grDevices::png"))) {


This seems a confusing interface, because it seems to imply that you can use any graphics device, but only png will work.

trevorld · 2023-10-17T18:27:19Z

This seems a confusing interface, because it seems to imply that you can use any graphics device, but only png will work.

I think most package developers who want to develop with R's bleeding edge graphic capabilities will simply replace agg::agg_png() with png(..., type="cairo") but if desired I could modify the code to allow even more graphics devices to record the plots.

Helpful information to know for such a change:
- {knitr}'s dev argument allows it to be a vector of multiple devices
  (images will be saved for each device).
  Is {pkgdown}'s dev argument also allowed to be a vector of multiple devices?
  If so should we assume we should record the plot with the first specified device?
- Is there a convenience function that converts a {pkgdown} dev string to
  a device function? Or perhaps should we use a big switch() statement of commonly used device names (defaulting to ragg::agg_png())?
- Paul Murrell seems to first implement bleeding edge R graphics features in the Cairo devices and pdf().
  Are there many {pkgdown} users that are putting pdf() images in their pkgdown websites?
  Or .bmp, .jpeg, or .tiff bitmap images instead of .png?

does this need changes to the documentation ...?

IMHO I think this would only warrant a NEWS.md entry and a mention in the release notes.

does this need changes to the ... tests?

Not entirely sure if you want to explicitly test this but here is an example that should show a blue oval if recorded and plotted with png(..., type = "cairo") that should
be blank if recorded or plotted with agg_png(...). This example uses the "affine transformation" feature introduced in R 4.2.0 which is supported in the R's Cairo devices and the pdf() device but not yet the {ragg} devices:

# Should draw a blue oval if device supports affine transformation feature
if (getRversion() >= '4.2.0' &&
    isTRUE(grDevices::dev.capabilities()$transformations) &&
    require("grid")) {
  define <- defineGrob(circleGrob(gp = gpar(col = NA, fill = "blue")), 
                       vp = viewport(width = unit(1, "in"), height = unit(1, "in")))
  grid.draw(define)
  grid.use(define$name, vp = viewport(width = unit(2, "in"), height = unit(1, "in")))
}

mjskay · 2024-02-21T00:32:55Z

Any movement on this? I'd be happy to take up the mantle of this PR if needed. It currently affects multiple packages I maintain (ggdist, ggblend) where documentation can't be rendered correctly by pkgdown because ragg does not support all modern {grid} features.

I'll second @trevorld that I'm not sure anything other than png really needs to be supported here (or maybe png and svg), and note that the existing interface is already confusing in that passing anything other than "ragg" seems to silently still use ragg. I think it would be worth a note in documentation about how this parameter doesn't work as you would expect it to if coming from rmarkdown.

hadley · 2024-02-21T13:25:51Z

@mjskay No updates, but I hope to turn my attention towards pkgdown in the not too distant future. But what features is ragg missing? I don't see any open issues in ragg that suggest there's big chunks missing, and I had thought we were up to date with base R features.

trevorld · 2024-02-21T16:04:25Z

But what features is ragg missing? I don't see any open issues in ragg that suggest there's big chunks missing, and I had thought we were up to date with base R features.

{ragg} supports all the features introduced in R 4.1 but not new graphic features introduced in later versions of R:

Introduced in R 4.2:

Luminance masks (although {ragg} does support alpha masks)
Compositing operators
Affine transformations
Stroking and filling paths

Introduced in R 4.3

Glyphs (although there is a year-old PR for this: suggestion for support of dev->glyph() ragg#123)

I think I recall a mention from the ragg maintainer a while ago that they were planning on supporting the R 4.2 features but were having difficulties with finalizing the compositing operator feature?

trevorld · 2024-02-21T17:28:07Z

In my case my packages {affiner} and {piecepackr} use the affine transformation feature introduced in R 4.2 (the latter since April, 2022) and this PR would allow me to render all my examples that use it.

hadley · 2024-02-21T17:41:12Z

@trevorld could you please file that as an issue on ragg?

mjskay · 2024-02-21T19:00:13Z

If the problem occurs not in the final rendering (which I understand already uses the requested device?), but only when the plot is recorded, would a simpler solution be to always record the plot using a device that will generally have the widest feature support --- e.g. svg or cairo png?

trevorld · 2024-02-21T19:15:06Z

would a simpler solution be to always record the plot using a device that will generally have the widest feature support --- e.g. svg or cairo png?

I believe you can build R without Cairo support and I think {ragg} might be faster than the Cairo devices so I'm not sure you'd want to always record with a Cairo device. Also there may be a future scenario where {ragg} supports luminance masks while the Cairo devices don't (I think only pdf() supports them for now although generally most new features are also implemented in the Cairo devices as well and pdf() has its own issues with throwing WARNINGs when trying to render UTF8 glyphs).

hadley · 2024-03-11T16:17:01Z

Do you have some time to think/talk about this this week? If so, we should be able to get something into the upcoming patch release.

To me, the main question is what the interface should look like. I think you proposal suggests an interface more broad than what we want to support, and something more like png: ragg, and png: base etc. (I don't think that's quite the right interface yet but it feels like it's heading in the right direction)

trevorld · 2024-03-11T17:46:39Z

To me, the main question is what the interface should look like. I think you proposal suggests an interface more broad than what we want to support, and something more like png: ragg, and png: base etc. (I don't think that's quite the right interface yet but it feels like it's heading in the right direction)

Here is a recap of my understanding of the current situation and this PR:

Currently there is no interface to change which graphics device records example plots (it is always ragg::agg_png()). This leads to certain examples getting skipped when {ragg} doesn't support the relevant feature and hence grDevices::dev.capabilities() indicates the example won't work.
There is an existing interface (documented here via the _pkgdown.yml file to change which graphic devices renders example plots i.e.

figures:
  dev: grDevices::png
  dev.args:
    type: "cairo"

will render (but not record) with the base Cairo png device instead of {ragg}.

What my PR currently does is if a user explicitly requests in the _pkgdown.yml to render with grDevices::png() that we also record with grDevices::png() as well.
@mjskay can chime in but I think this solves our problem and doesn't require an interface change.
- Note though if a user specifies an alternative graphics device to {ragg} that isn't a base png device we continue to record with {ragg}. It is unclear to me how many developers will want to a use a feature not supported by {ragg} and won't be picking the base Cairo png device...
However if you want {pkgdown} could certainly have a separate new option to customize which device records the examples. Note that base R has several png type's i.e. "cairo", "Xlib", "quartz", "windows" and some of the newest grid graphics features are only supported in the "cairo" type but R isn't guaranteed to be compiled with Cairo support (or png support at all). So png: base seems like it won't be specific enough, we'd want some way to ensure we're also getting the right type.
- Theoretically I suppose you could even want to record the examples with a non-png device but I'm not sure under what circumstances someone would need to use this in practice.

hadley · 2024-03-11T18:27:21Z

Oh I didn't realise that the problem was only with recording. In that case, is it possible to take a more general approach and just record and replay with the same device? That is what I'd expect.

trevorld · 2024-03-11T18:39:59Z

In that case, is it possible to take a more general approach and just record and replay with the same device? That is what I'd expect.

This would be best if "easy". Things that may or may not be "easy":

Is there an easy way to turn all legal {pkgdown} configuration dev strings into graphics device functions (with relevant arguments like type passed in)?
Are there ever situations with {pkgdown} (like sometimes occurs with {knitr}) where a user may ask to render with multiple devices? In that case which one should be used to record?

I asked about these in an earlier comment in this PR...

hadley · 2024-03-11T21:44:22Z

match.fun() + do.call()? Or instead of match.fun() maybe just parse + eval (since that will also support ::)? If you can sketch our something basic, I'm happy to fill in any missing pieces for more complicated scenarios.

I think we could error if there are multiple devices supplied, or simply consider that out of scope.

mjskay · 2024-03-11T22:16:44Z

Note though if a user specifies an alternative graphics device to {ragg} that isn't a base png device we continue to record with {ragg}. It is unclear to me how many developers will want to a use a feature not supported by {ragg} and won't be picking the base Cairo png device...

FWIW I have been using svg in ggblend, though I'd be fine with using png there.

I agree that just recording with the correct device would be nice.

trevorld · 2024-03-12T17:21:05Z

Do you have some time to think/talk about this this week? If so, we should be able to get something into the upcoming patch release.

If you can sketch our something basic, I'm happy to fill in any missing pieces for more complicated scenarios.

I got some urgent work for today but I should be able to update my PR later this week.

hadley · 2024-03-12T18:44:41Z

Looks like match_fun() already does the processing needed to handle different dev specs and give a reasonable error if not appropriate type.

hadley · 2024-03-12T18:52:31Z

So the code might look something like this (untested):

settings <- fig_settings()
dev <- match_fun(settings$dev %||% ragg::agg_png)
settings$dev <- NULL
device <- function(...) {
  args <- modify_list(settings, list(...))
  do.call(dev, args)
}

Hmmmm, that's not quite right because the fig_settings() are the knitr options. Maybe you only need to hoist out other.parameters and bg? It seems like it might be worth also setting the width and height, since I think that would save one re-layout.

trevorld · 2024-03-12T19:01:02Z

For the base bitmap devices you'll also want to grab fig_settings()$dev.args$type if it was defined since it matters if we are using the "cairo" device versus say the "windows" device.
I'm not sure what situation the bg would have an effect when recording a plot. Maybe the width and height might matter in a rare edge case.
We may also need to filter out the arguments in args that aren't in the formals(dev)

hadley · 2024-03-12T20:23:48Z

Oh yeah, I meant fig_settings()$dev.args, not other.parameters.

I think the main advantage of setting width and height would be minor performance improvement since (presumably) we'd only need to lay out the plot once, not twice. But I have no idea if this is true or if it's an expensive operation or not.

I'd prefer to not filter out arguments based on formals unless we absolutely have to, as that's an easy place to bugs to silently hide.

thomasp85 · 2024-03-13T08:35:11Z

Just a small data point - ragg with support for all the graphic engine features will land on CRAN in a few days... There may still be good reasons to use other graphics devices so maybe this PR is still relevant, but the original issue should disappear very soon

trevorld · 2024-03-13T16:31:48Z

Just a small data point - ragg with support for all the graphic engine features will land on CRAN in a few days... There may still be good reasons to use other graphics devices so maybe this PR is still relevant, but the original issue should disappear very soon

I've installed the development version of {ragg} and confirm that {pkgdown} no longer skips my examples when recorded by {ragg}. Everything now works fine for my purposes.
Previously when R 4.1/4.2/4.3 introduced new graphics features it took several months for them to get supported by {ragg} so theoretically in the future a similar issue may affect bleeding edge graphics package developers if we always record with the {ragg} device. However Paul Murrell's website isn't suggesting any new graphic feature in the pipeline for R 4.4 so this doesn't seem like it would be relevant until R 4.5 at the earliest (to be released circa April 2025).
On the flipside continuing with this PR risks introducing a new bug into {pkgdown} with no longer a known short-term benefit.

@hadley let me know if you'd still like me to work on this PR. I'm okay if it gets closed due to the recent {ragg} update.

hadley · 2024-03-13T19:59:50Z

If the latest version of ragg fixes your problem, I'm happy to close this PR since it's less work for me 😁

trevorld mentioned this pull request Jun 1, 2023

Get affine transformation example plots to work in {pkgdown} trevorld/affiner#34

Closed

hadley reviewed Oct 17, 2023

View reviewed changes

trevorld mentioned this pull request Dec 8, 2023

Integrating new grid features in ggplot2 tidyverse/ggplot2#3997

Closed

trevorld mentioned this pull request Feb 21, 2024

affine transformation feature r-lib/ragg#157

Closed

hadley added this to the 2.0.8 milestone Mar 11, 2024

hadley closed this Mar 13, 2024

mjskay mentioned this pull request Dec 16, 2024

Does ggblend work with pkgdown? mjskay/ggblend#25

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Record example plots with 'png()' if requested #2314

Record example plots with 'png()' if requested #2314

trevorld commented Jun 1, 2023 •

edited

Loading

hadley left a comment

hadley Oct 17, 2023

trevorld commented Oct 17, 2023 •

edited

Loading

mjskay commented Feb 21, 2024 •

edited

Loading

hadley commented Feb 21, 2024

trevorld commented Feb 21, 2024 •

edited

Loading

trevorld commented Feb 21, 2024

hadley commented Feb 21, 2024

mjskay commented Feb 21, 2024

trevorld commented Feb 21, 2024

hadley commented Mar 11, 2024

trevorld commented Mar 11, 2024

hadley commented Mar 11, 2024

trevorld commented Mar 11, 2024

hadley commented Mar 11, 2024

mjskay commented Mar 11, 2024

trevorld commented Mar 12, 2024 •

edited

Loading

hadley commented Mar 12, 2024

hadley commented Mar 12, 2024 •

edited

Loading

trevorld commented Mar 12, 2024

hadley commented Mar 12, 2024

thomasp85 commented Mar 13, 2024

trevorld commented Mar 13, 2024

hadley commented Mar 13, 2024

Record example plots with 'png()' if requested #2314

Record example plots with 'png()' if requested #2314

Conversation

trevorld commented Jun 1, 2023 • edited Loading

hadley left a comment

Choose a reason for hiding this comment

hadley Oct 17, 2023

Choose a reason for hiding this comment

trevorld commented Oct 17, 2023 • edited Loading

mjskay commented Feb 21, 2024 • edited Loading

hadley commented Feb 21, 2024

trevorld commented Feb 21, 2024 • edited Loading

trevorld commented Feb 21, 2024

hadley commented Feb 21, 2024

mjskay commented Feb 21, 2024

trevorld commented Feb 21, 2024

hadley commented Mar 11, 2024

trevorld commented Mar 11, 2024

hadley commented Mar 11, 2024

trevorld commented Mar 11, 2024

hadley commented Mar 11, 2024

mjskay commented Mar 11, 2024

trevorld commented Mar 12, 2024 • edited Loading

hadley commented Mar 12, 2024

hadley commented Mar 12, 2024 • edited Loading

trevorld commented Mar 12, 2024

hadley commented Mar 12, 2024

thomasp85 commented Mar 13, 2024

trevorld commented Mar 13, 2024

hadley commented Mar 13, 2024

trevorld commented Jun 1, 2023 •

edited

Loading

trevorld commented Oct 17, 2023 •

edited

Loading

mjskay commented Feb 21, 2024 •

edited

Loading

trevorld commented Feb 21, 2024 •

edited

Loading

trevorld commented Mar 12, 2024 •

edited

Loading

hadley commented Mar 12, 2024 •

edited

Loading