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

Device capabilities checker #5350

Merged
merged 20 commits into from
Oct 2, 2023
Merged

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Jul 13, 2023

This PR aims to fix #5332.

It resolves a roadblock for #5299 and an exported check utility for testing the capabilities of the graphics device.
A few notes:

  • It is not vectorised: only one feature can be probed at a time.
  • Supports R 4.2.0+ features too, like compositing, luminance masks and glyphs.
  • The {vdiffr} issue is unresolved and {svglite} assumptions are used.
  • I had to use a lot of mocking in the tests, as I cannot reproduce every condition on GHA.
  • Gives FALSE for patterns and gradients in R 4.1.0 because they can't be vectorised.
  • In general, if devices aren't configured to make use of dev.capabilities(), the guesses remain sketchy. This is particularly the case in R 4.1.0 and even more so for raster devices on Windows platforms.

A small demo running this on R 4.2.0, which supports most features but not glyphs. You can use check_device() in control flow statements when action = "test".

devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2
withr::local_pdf()
getRversion()
#> [1] '4.2.0'

if (check_device("patterns", action = "test")) {
  print("Yay")
} else {
  print("Nay")
}
#> [1] "Yay"

If action = "warn" it warns with the likely reason why a feature isn't available:

if (check_device("compositing", action = "warn")) {
  print("Yay")
} else {
  print("Nay")
}
#> Warning: The pdf device does not support compositing.
#> [1] "Nay"

If action = "error" an error is thrown. Here, because of a version mismatch.

if (check_device("glyphs", action = "abort")) {
  print("Yay")
}
#> Error:
#> ! R 4.2.0 does not support glyphs.

Created on 2023-07-13 with reprex v2.0.2

@teunbrand
Copy link
Collaborator Author

@mjskay because you were rooting for a checker in #5299 (comment): do you think the proposed solution will help ggdist/ggblend? I'd be happy to make improvements if needed

@mjskay
Copy link
Contributor

mjskay commented Jul 18, 2023

Awesome, thanks @teunbrand! I'll try to take a look this week and let you know what I think. Really excited to offload this logic :).

@mjskay
Copy link
Contributor

mjskay commented Jul 31, 2023

This is great, thanks!

Some thoughts:

  • This does not appear to give me correct results with "RStudioGD" in windows (i.e., when I use it in the R Console in RStudio). I think this is because the RStudio graphics device does not correctly report the capabilities of whatever underlying device is in use. So, for example, I can output blends and gradients if I select a device in the RStudio settings that supports blends or gradients, but in the console I get warnings:

    > ggplot2::check_device("gradients")
    [1] FALSE
    Warning message:
    Unable to check the capabilities of the RStudioGD device. 
    > ggplot2::check_device("blending")
    [1] FALSE
    Warning message:
    The RStudioGD device does not support blend modes. 

    Not sure if that can be fixed here or if that's just an RstudioGD bug.

  • It occurs to me that it might be useful to distinguish between three possible outcomes of the check: definitely supports a feature, definitely does not support a feature, and maybe supports a feature. E.g. in ggdist, it would be useful for me to be able to apply more strict checks for gradient support, using a fallback with a warning only in the "maybe" case, but allowing the user to override this fallback manually if they do have gradient support. Maybe this is a good use of three-valued logics? E.g. the return could be TRUE, FALSE, or NA (definitely yes, definitely no, maybe), and then a pattern of isTRUE(check_device(...)) would be "definitely yes", isFALSE(check_device(...)) would be "definitely no", and !isFALSE(check_device(...)) would be "maybe or yes"?

  • Another thing to consider is how this plays with package's custom warnings that provide additional context / information. E.g. here is an example warning from ggblend:

    Warning message:
    Your graphics device, "RStudioGD", reports that blend = "multiply" is not supported.
     - If the blending output IS NOT as expected (e.g. geoms are not being
       drawn), then you must switch to a graphics device that supports blending,
       like png(type = "cairo"), svg(), or cairo_pdf().
     - If the blending output IS as expected despite this warning, this is
       likely a bug *in the graphics device*. Unfortunately, several graphics do
       not correctly report their capabilities. You may wish to a report a bug
       to the authors of the graphics device. In the mean time, you can disable
       this warning via options(ggblend.check_blend = FALSE).
     - For more information, see the Supported Devices section of help('blend'). 
    

    and an example warning from ggdist (when fill_type = "auto"):

    `fill_type = "gradient"` is not supported by the current graphics device, which is
    `"RStudioGD"`.
    ℹ Falling back to `fill_type = "segments"`.
    ℹ If you believe your current graphics device does support `fill_type = "gradient"`
      but auto-detection failed, try setting `fill_type = "gradient"` explicitly. If this
      causes the gradient to display correctly, then this warning is likely a false
      positive caused by the graphics device failing to properly report its support for
      the `"LinearGradient"` pattern via `grDevices::dev.capabilities()`. Consider
      reporting a bug to the author of the graphics device.
    ℹ See the documentation for `fill_type` in `ggdist::geom_slabinterval()` for more
      information. 
    

    It would be great if it was easy for the first part of the warning to be supplied by the check function, then I could give some additional context for how to address the problem for the geom at hand.

  • Currently, ggblend checks that the specific blend / compositing mode is supported by the device, not just "blending" or "compositing" in general. See e.g. here. Not sure if that would be easy to support or not. Worst case I can rewrite it to check generically for blending/compositing support, it's not a huge deal.

@teunbrand
Copy link
Collaborator Author

Thanks so much for your thoughts @mjskay!

the RStudio graphics device does not correctly report the capabilities

Yeah I struggled with this as well. I think whenever RStudioGD is the active device, the next device is the back-end, but I don't know how well that assumption holds. In any case, that is what the latest changes check for now.

Maybe this is a good use of three-valued logics?

I see your points, and I think they make sense. The goal here was to make a checker that you could throw into an if-statement without any headaches, and returning NAs for unknown cases would defeat that purpose somewhat. I'd have to mull it over a bit.

how this plays with package's custom warnings

Yeah that is a good point. At first I thought I should implement a action = "text" option, but then I had realised you could just append warnings with calling handlers.

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

pdf()

append_warning <- function(expr, append = NULL){
  try_fetch(
    expr,
    warning = function(cnd) {
      warn(
        message = cnd_message(cnd),
        body    = cli::format_warning(append), 
        class   = class(cnd),
        call    = cnd$call
      )
      invokeRestart("muffleWarning")
    }
  )
}

append_warning(
  check_device("compositing", action = "warn", op = "clear"),
  c("*" = "Thou shalt not use this function.")
)
#> Warning: The pdf device does not support 'clear' compositing.
#> • Thou shalt not use this function.
#> [1] FALSE

Created on 2023-07-31 with reprex v2.0.2

checks that the specific blend / compositing mode is supported by the device

As a device can potentially support an arbitrary subset of these blending/compositing operations, this seems like a good thing to include. I've included an op argument to check a specific operation (also see reprex above).

@mjskay
Copy link
Contributor

mjskay commented Aug 1, 2023

Yeah I struggled with this as well. I think whenever RStudioGD is the active device, the next device is the back-end, but I don't know how well that assumption holds. In any case, that is what the latest changes check for now.

Ah great! FWIW I tested the new version on RStudioGD on a few machines and it worked :). Thanks!

Yeah that is a good point. At first I thought I should implement a action = "text" option, but then I had realised you could just append warnings with calling handlers.

Nice, that should work for my needs and is more elegant :)

I see your points, and I think they make sense. The goal here was to make a checker that you could throw into an if-statement without any headaches, and returning NAs for unknown cases would defeat that purpose somewhat. I'd have to mull it over a bit.

Fair. As another suggestion, what about an argument like maybe with values "pass" (treat it as TRUE), "warn" (throw warning) and "error" (throw error). That gives finer control other the "maybe"s without invoking NA. E.g. I might use check_device("gradients", action = "test", maybe = "warning") or check_device("gradients", action = "error", maybe = "warning") in some parts of my code to get the desired behavior.

As a device can potentially support an arbitrary subset of these blending/compositing operations, this seems like a good thing to include. I've included an op argument to check a specific operation (also see reprex above).

Great, thanks! One minor issue with this is that it distinguishes between blending and compositing ops in the check, even though the underlying grid functions don't, which feels a bit awkward:

> ggplot2::check_device("blending", op = "multiply")
[1] TRUE
> ggplot2::check_device("compositing", op = "multiply")
Error in `ggplot2::check_device()`:
! `op` must be one of "clear", "source", "over", "in", "out", "atop", "dest",
  "dest.over", "dest.in", "dest.out", "dest.atop", "xor", "add", or "saturate", not
  "multiply".
Run `rlang::last_trace()` to see where the error occurred.

For my usage, ideally I'd like to be able to just check for support for an op without worrying if it is a blending or a compositing op.

Thanks for all the work on this!

@teunbrand
Copy link
Collaborator Author

Latest changes:

  • The {vdiffr} issue is resolved because the device name can now be discriminated.
  • Internally, composition and blend operations are treated the same, the difference is only relevant for the messaging.
  • I added a maybe argument that can take a boolean. So if you want to try the device, but warn about it, you can use action = "warn", maybe = TRUE.

@thomasp85 thomasp85 self-requested a review September 12, 2023 09:16
Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@teunbrand
Copy link
Collaborator Author

Thanks so much @mjskay for all the feedback on this PR!

@teunbrand teunbrand merged commit 89204bc into tidyverse:main Oct 2, 2023
11 of 12 checks passed
@teunbrand teunbrand deleted the device_capabilities branch October 2, 2023 13:13
@mjskay
Copy link
Contributor

mjskay commented Oct 2, 2023

Of course! Thanks for all the hard work and iteration. It's definitely gonna make my life easier :)

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

Successfully merging this pull request may close these issues.

Feature request: graphics device capabilities checker
3 participants