Prevent `strwidth()` and `par()` calls from leaking graphics devices

[nanxstats]

Fixes #227

This is new try (after PR #228) to fix the issue reported in #227 where calls to graphics::strwidth() and graphics::par() unintentionally generate Rplots.pdf files in some environments due to opening a new default graphics device that is never closed.

The discussion in that PR was very helpful in getting this "more proper" solution.

• Add a with_graphics_device() helper to run graphics::strwidth()/graphics::par() without implicitly leaving the default device open by opening a null-output device only when needed and closing it on exit.
• Refactor code under R/ and tests/ to use the helper.

Performance comparison:

microbenchmark::microbenchmark(
  r2rtf:::with_graphics_device(graphics::strwidth("Hello world!", units = "inches", cex = 1, font = 1, family = "sans")),
  graphics::strwidth("Hello world!", units = "inches", cex = 1, font = 1, family = "sans"),
  times = 10000
)

#>   min    lq     mean median    uq    max neval cld
#> 4.305 4.715 5.132995  5.002 5.371 33.579 10000   a
#> 1.968 2.214 2.418569  2.337 2.501 26.855 10000   b

Note that changing this might affect some strwidth() calculation results slightly.

[elong0527]

Cool, thanks for fixing the issue!

[yihui]

Just to make sure I understand rtf_strwidth() correctly: this function measures the width of strings under different settings of cex, font, and family. It seems that the only possible way to do it in base R is through a graphical device. My question is, is this measurement dependent on the current device? My intuition is no (and I hope so, otherwise we'll be in trouble). If that's the case, I don't quite understand the complexity of this PR. Why not just

pdf(NULL)
on.exit(dev.off())

in rtf_strwidth()?

[yihui]

is this measurement dependent on the current device? My intuition is no (and I hope so, otherwise we'll be in trouble)

Unfortunately, a quick test shows that I was wrong:

> png()
> strwidth('hello world', 'inches', family = 'Times')
[1] 0.7637533
> dev.off()

> pdf()
> strwidth('hello world', 'inches', family = 'Times')
[1] 0.7621667
> dev.off()

That means rtf_strwidth() is dependent on the current device, which will make its output unpredictable...

[yihui]

BTW, although I don't think any of you would consider switching to another testing framework, I'd like to mention that I also ran into this problem several years ago and provided an option in {testit} to automatically clean up new files generated from tests (including this Rplots.pdf): https://github.com/yihui/testit/blob/1f11885b/R/testit.R#L168-L172

[nanxstats]

Yes, it seems clear that the original intention for strwidth() and par() is for users to only use them within a graphical device context. So a simple solution like pdf()-dev.off() would make sense - although that would change the existing calculations a bit, which may impact some line breaking and pagination results. Maybe this is acceptable.

And yes, the current output is dependent on the graphical device context and thus unpredictable. The goal of this PR is to eliminate the annoying Rplot.pdf behavior but not fixing things fundamentally - because that would involve either introducing systemfonts::string_width() as a hard dependency (the target is zero), or re-implementing a similar function from scratch by adding SystemRequirements: freetype2 and writing C wrappers. I can try that too, but it will add additional burdens of compiling this package from source.

Theoretically, calculating this should NOT require a graphical device. The parameters that truly matter are the font, font size, and DPI. See a proper solution in rtflite (Python) calling Pillow's ImageFont.getlength method which ultimately uses FreeType to do this calculation.

Here is a table comparing these possible solutions:

Dimension	PR#285	pdf()	FreeType2	systemfonts
Avoids Rplots.pdf leak	Yes	Yes	Yes	Yes
Backward compatible results	Yes	No	No	No
Absolutely correct results	No	No	Yes	Yes
Predictable across environments	No	No	Yes	Yes
Zero dependencies	Yes	Yes	Yes	No
No compile-time burden	Yes	Yes	No	No

[nanxstats]

I'm merging this as this seems to be the optimal solution at the moment without moving mountains.