r-lib / vdiffr

Visual regression testing and graphical diffing with testthat
https://vdiffr.r-lib.org
Other
180 stars 31 forks source link

Update embedded svglite to v2.10+ to gain pattern support. #132

Open coolbutuseless opened 1 year ago

coolbutuseless commented 1 year ago

The current embedded version of svglite within this package does not appear to be in sync with the latest svglite release.

E.g. Latest svglite (v2.1.1) includes support for pattern and gradient fills (included in v2.1.0).

i.e. vdiffr does not support pattern rendering when comparing images.

Would this just be the case of copying over the latest svglite release?

What is the reason for embedding a local version rather than linking to svglite package?

lionel- commented 1 year ago

The reason is stability of SVG generation. Updating svglite may require vdffir users to update their snapshots.

trevorld commented 1 year ago

I think {vdiffr} does allow the developer the option to link to the {svglite} package directly via expect_doppelganger()'s writer argument (although this could be made easier possibly by exporting print_plot()):

write_svg_bleeding_edge <- function(plot, file, title = "") {
  svglite::svglite(file)
  on.exit(grDevices::dev.off())
  # warning: `print_plot()` is not exported by {vdiffr}
  vdiffr:::print_plot(plot, title) 
}
expect_doppelganger(plot_fn(), writer = write_svg_bleeding_edge)

I've experienced pain when developing {gridpattern} in trying tell apart if a pattern drawing function is being called from svglite::svglite() (which supports patterns) and vdiffr::svglite() (which doesn't) which will hopefully be fixed either by https://github.com/r-lib/svglite/issues/152 or {vdiffr} upgrading to a version of {svglite} that supports patterns.

mjskay commented 1 year ago

FWIW, if anyone else is encountering this problem (ggdist has the same problem b/c I want to test gradient support with vdiffr), I've been using a modified version of @trevorld's writer to generate SVGs that work across Windows, Linux, and Mac (use of Liberation Sans and Symbola is cribbed from what vdiffr does internally):

write_svg_bleeding_edge = function(plot, file, title = "") {
  # use Liberation Sans and Symbola to avoid platform-specific font differences
  liberation_sans = fontquiver::font_styles("Liberation", "Sans")
  symbola = fontquiver::font("Symbola", "Symbols", "Regular")
  sysfonts::font_add(
    "Liberation Sans",
    regular = liberation_sans$Regular$ttf,
    bold = liberation_sans$Bold$ttf,
    italic = liberation_sans$Italic$ttf,
    bolditalic = liberation_sans$`Bold Italic`$ttf,
    symbol = symbola$ttf
  )

  svglite::svglite(file)
  showtext::showtext_begin()
  on.exit({
    showtext::showtext_end()
    grDevices::dev.off()
  })

  print(
    plot + ggtitle(title) + theme_test(base_family = "Liberation Sans")
  )
}
teunbrand commented 1 year ago

The reason is stability of SVG generation.

I find this argument very reasonable, but is it an option to change the device name from "devSVG" to something else so that we can check whether the {svglite} or {vdiffr} device is active through names(dev.cur())? ggplot2 is also aiming to implement some basic pattern functionality.

EDIT: considered fixed by #135

teunbrand commented 2 weeks ago

In addition to pattern support, it would be nice to have support for other newer graphical features as well (clipping paths, masks, compositing/blending, affine transformations, glyphs and fill/stroke paths).