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

Barebones support for <GridPattern> fills. #5299

Merged
merged 25 commits into from
Dec 8, 2023

Conversation

teunbrand
Copy link
Collaborator

This PR sprang forth from #3997.

Briefly, it attempts to provide some barebones support for {grid}'s new pattern features.

Less briefly:
Essentially, the only piece of ggplot2's internals that hampered using grid's patterns was the alpha() function that has restrictions on what it considers a colour. To lift this restriction, there is now a fill_alpha() function exported (for extension devs), that can set alpha on any of {grid}'s 3 pattern classes.

What can you do with this PR:

  • Set geom_rect(fill = <(list of) GridPattern>) outside the aes().
  • Set geom_rect(aes(fill = something)) + scale_fill_manual(values = <list of GridPattern>).

What this PR doesn't do:

  • Provide scale infrastructure for mapping patterns.
  • Provide additional aesthetics for patterns.

I'm also pinging @trevorld and @coolbutuseless because they might have some opinions about this PR and experience with patterns.

A small demo:

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

# Borrowed straight from the README of https://github.com/clauswilke/ggtextures
images <- c(
  "http://www.hypergridbusiness.com/wp-content/uploads/2012/12/rocks2-256.jpg",
  "http://www.hypergridbusiness.com/wp-content/uploads/2012/12/stone2-256.jpg",
  "http://www.hypergridbusiness.com/wp-content/uploads/2012/12/siding1-256.jpg"
)

patterns <- lapply(images, function(url) {
  img <- grid::rasterGrob(
    magick::image_read(url),
    width  = unit(2, "cm"),
    height = unit(2, "cm")
  )
  grid::pattern(img, width = img$width, height = img$height, extend = "repeat",
                group = FALSE)
})

ggplot(mpg, aes(class)) +
  geom_bar(aes(fill = factor(drv)), colour = 'black') +
  scale_fill_manual(values = patterns)

Created on 2023-05-06 with reprex v2.0.2

R/utilities-patterns.R Outdated Show resolved Hide resolved
R/utilities-patterns.R Outdated Show resolved Hide resolved
@teunbrand
Copy link
Collaborator Author

Alright, time for a small update.

  • I've switched alpha masks for tiled patterns to use white as the base colour.
  • It now uses the new check_device() in appropriate places.

Remaining issues:

  • For tile patterns based on rasterGrob(), we currently do not modify the colours directly. Doing so would require farver::multiply_channel(), and {farver} is not a direct dependency (it is via {scales}).
  • Should we make pattern_alpha() an S3 generic + methods?

Some examples for all pattern options:

Linear gradients work as expected.

devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2
#> Warning: package 'testthat' was built under R version 4.3.1

p <- ggplot(mpg, aes(drv))

grad <- linearGradient(rainbow(10))
p + geom_bar(fill = grad, alpha = 0.5)

Radial gradients work as expected.

grad <- radialGradient(rainbow(10))
p + geom_bar(fill = grad, alpha = 0.5)

Tiled patterns work as expected.

tile <- pattern(
  circleGrob(r = unit(1, "mm"), gp = gpar(fill = "black")), 
  extend = "repeat", width = unit(3, "mm"), height = unit(3, "mm")
)
p + geom_bar(fill = tile, alpha = 0.5)

The {gridpattern} patterns work if wrapped in grid::pattern().

pattern <- pattern(gridpattern::grid.pattern_circle(
  density = 0.5, grid = "hex_circle", fill = c("blue", "yellow", "red"), 
  draw = FALSE
))
p + geom_bar(fill = pattern, alpha = 0.5)

Example of error message when device doesn't support feature:

png(type = "windows")
p + geom_bar(fill = pattern, alpha = 0.5)
#> Error in `geom_bar()`:
#> ! Problem while converting geom to grob.
#> ℹ Error occurred in the 1st layer.
#> Caused by error in `fill_alpha()` at ggplot2/R/geom-rect.R:54:6:
#> ! Unable to check the capabilities of the png device.
dev.off()

Created on 2023-10-06 with reprex v2.0.2

@brunomioto
Copy link

brunomioto commented Oct 6, 2023

Alright, time for a small update.

Awesome update @teunbrand !

It's possible to edit the angle of linear gradients?

@teunbrand
Copy link
Collaborator Author

It's possible to manage the angle of linear gradients?

Yes, but that all happens in grid::linearGradient(), specifically by tweaking the x1,x2,y1 and y2 arguments, but that isn't something that ggplot2 should worry about.

@trevorld
Copy link

trevorld commented Oct 7, 2023

Looks good!

I observe that fill_alpha() throws an error if you give it a length-one list with a color vector of length greater than one:

> fill_alpha(list(c("blue", "red")), 0.5)
Error in `fill_alpha()`:
! fill must be a valid colour or list of <GridPattern> objects.
Run `rlang::last_trace()` to see where the error occurred.

Future extension authors perhaps may want it to support color vectors greater than one? A future release of {ggpattern} will probably allow the pattern_fill aesthetic to be lists of color vectors greater than one although in this case the alpha adjustment may be handed off to {gridpattern}.

@teunbrand
Copy link
Collaborator Author

Ah right, perhaps the error message should be clearer, but you can give it a bare vector of colours, they don't need to be in a list.

fill_alpha(c("blue", "red"), 0.5)
> [1] "#0000FF80" "#FF000080"

@trevorld
Copy link

Ah right, perhaps the error message should be clearer, but you can give it a bare vector of colours, they don't need to be in a list.

I observe that if you do something like geom_custom(fill = list("grey", c("white", "black")) that the second c("white", "black") fill will often be seen as something like list(c("white", "black")) in the Geom's custom draw_group() method (where an extension developer is likely to call fill_alpha()) instead of c("white", "black").

I understand that in the base {ggplot2} geoms such a two-element fill may not make sense but a multiple-color pattern_fill should make sense (in the future) for {ggpattern} and perhaps other future extensions.

Thought that maybe this would be an additional special case it may be nice for fill_alpha() to handle so an extension developer wouldn't need to detect it fill was a list of colors and if so unwrap() it and then call fill_alpha()?

@teunbrand
Copy link
Collaborator Author

teunbrand commented Oct 11, 2023

Thought that maybe this would be an additional special case it may be nice for fill_alpha() to handle so an extension developer wouldn't need to detect it fill was a list of colors and if so unwrap() it and then call fill_alpha()?

I can see the utility of that but I'm having a hard time imagining why you'd need fill_alpha() in that case. Ideally developers would only use fill_alpha() if they allow the users to give patterns for their geoms as the fill aesthetic. If you have a ragged list of plain colours (to compose your own patterns), you might as well stick to regular scales::alpha(). Unless you're thinking about nesting patterns in patterns?

@trevorld
Copy link

If you have a ragged list of plain colours (to compose your own patterns), you might as well stick to regular scales::alpha(). Unless you're thinking about nesting patterns in patterns?

  1. scales::alpha() currently throws an error if you give it a length-one list with a color vector of length strictly greater than one.
  2. The immediate goal is to allow {ggpattern} users to directly compose multi-color patterns in {ggplot2} without messing with custom patterns but I think I should allow nesting patterns in patterns in the future.
  3. I'm going to want this lower level stuff to happen in {gridpattern} which I don't want to depend on {ggplot2} so I'm probably not going to want to directly call ggplot2::fill_pattern() but I may want to copy and paste a fork of the functions (and update the DESCRIPTION/LICENSE appropriately).

@teunbrand
Copy link
Collaborator Author

Alright I have given it some more consideration, but the most important thing to me is that fill_alpha() delivers output that is valid input for gpar(fill = ...). A <list> of colours is not valid input for gpar():

grid::gpar(fill = c("red", "blue"))
#> $fill
#> [1] "red"  "blue"
grid::gpar(fill = list("red", "blue"))
#> Error in validGP(list(...)): 'fill' gpar list components must all be patterns
grid::gpar(fill = list(list("red", "green"), "blue"))
#> Error in validGP(list(...)): 'fill' gpar list components must all be patterns

Created on 2023-10-12 with reprex v2.0.2

This PR is allowing mixed inputs of list(<pattern>, <character_colour>) for user convenience, in which case the character colour is converted to a pattern. However, geoms are free to slice the input data to their whims, which means some Geom$draw_group() method (or legend keys), can have list(<character_colour>) as input to fill_alpha(). To deliver compatible output with gpar(), fill_alpha() should unlist() such cases. In other words, fill_alpha() is sacrificing type stability to minimise fuss with gpar(). If we can convince the {grid} package to support mixed input, it would be no problem to handle ragged lists* here.

* I'm using the term 'ragged list' to mean a list where lengths(my_list) can be any sequence of positive integers (and 0).

Now, aside from the problem that ragged lists of colours aren't valid input for gpar(), the need to unlist() lists of colours might give unexpected results. For example, if your 'ragged' list actually turns out to be a flat list where all(lengths(x) == 1), then you get a plain character vector because we need valid gpar() input. However if your ragged list is truly ragged, you would get a list back where all(lengths(input) == lengths(output)) (if fill_alpha() was adjusted to add support for such cases). This would require some defensive programming on the extension developers part to deal with the type instability.

Lastly, and this is more of a minor point, if fill_alpha() were to handle a ragged list of colours, then it should also support a ragged list of alphas and it gets tedious to throw the appropriate errors if these are misaligned.

Thus, my advice for ragged lists is to handle these in your package according to your needs. ggplot2 is MIT licenced so you're free to adapt fill_alpha() for your purposes if you want to support ragged lists.

R/geom-tile.R Show resolved Hide resolved
R/utilities-patterns.R Outdated Show resolved Hide resolved
R/utilities-patterns.R Show resolved Hide resolved
@thomasp85
Copy link
Member

This should probably live in another PR, but check_device() should exit early if no device is open

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 for the review Thomas!

@teunbrand teunbrand merged commit ad540b7 into tidyverse:main Dec 8, 2023
12 checks passed
@teunbrand teunbrand deleted the pattern_fills branch December 8, 2023 09:14
@teunbrand teunbrand mentioned this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement layers 📈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants