gganimate icon indicating copy to clipboard operation
gganimate copied to clipboard

generic device support

Open coolbutuseless opened this issue 6 years ago • 6 comments

This is a rough implementation of code to allow for more output devices (See Issue #318).

The user already specifies the character string for the 'device'. Rather than the existing 'switch()' statement to pick the device, use 'get()'.

Also allow the user to explicitly specify the file suffix if wanted, but by default choose the natural suffix for the devices that it knows about already.

coolbutuseless avatar May 05 '19 04:05 coolbutuseless

Nice one @coolbutuseless!

One comment: I tried to test this branch and found I couldn't use Cairo::CairoPNG as a device because device is converted to lower case here in prepare.args:https://github.com/thomasp85/gganimate/blob/9fe8335b89f58dd1cc623df10b34054393d2af4d/R/animate.R#L220 and thus get can't find 'cairopng'.

Move that into the switch maybe?

@thomasp85, I'm not sure if you're aware but R graphics look pretty awful out of box on Windows due to lack of anti-aliasing on devices that are supported there. One way around this is to use the devices in the Cairo package.

It would be really sweet to be able to set this as my device with gganimate and get anti-aliasing in my animations. Here's a quick example I made that's looking pretty retro: anim_no_anti_alias

MilesMcBain avatar Aug 05 '19 04:08 MilesMcBain

I made a quick change to @coolbutuseless's branch to show you the difference, rendering with Cairo::CairoPNG on Windows: anim_with_anti_aliasing

In doing so I noticed one additional flaw this implementation which is you need have the library loaded that contains the device i.e. get("Cairo::CairoPNG") doesn't work while get("CairoPNG") does.

Edit: ragg::agg_png also looking very spiffy 👍

MilesMcBain avatar Aug 05 '19 04:08 MilesMcBain

Can you not already use the Cairo device using the advice here, using type=“cairo” in the animate call? https://github.com/ropenscilabs/learngganimate/blob/master/animate.md#type-argument-why-your-animation-is-pixelated-on-windows

On Sun, Aug 4, 2019 at 9:44 PM Miles McBain [email protected] wrote:

I made a quick change to @coolbutuseless https://github.com/coolbutuseless's branch to show you the difference, rendering with Cairo::CairoPNG on Windows: [image: anim_with_anti_aliasing] https://user-images.githubusercontent.com/9996346/62439342-d9461a80-b78e-11e9-9e8d-eb4797a0c52c.gif

In doing so I noticed one additional flaw this implementation which is you need have the library loaded that contains the device i.e. get("Cairo::CairoPNG") doesn't work while get("CairoPNG") does.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/thomasp85/gganimate/pull/319?email_source=notifications&email_token=AEOX2Y7ABFQMH4SOCWLLOYLQC6VY7A5CNFSM4HK2K2Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QVIUY#issuecomment-518083667, or mute the thread https://github.com/notifications/unsubscribe-auth/AEOX2Y6IZRH2GKNJ7SKPVUDQC6VY7ANCNFSM4HK2K2ZQ .

jonspring avatar Aug 05 '19 07:08 jonspring

@jonspring that's an interesting suggestion I didn't know you could instantiate a Cairo device on Windows using an arg to dev().

I think this PR still stands regardless and the bugs I found still need to be addressed.

Also maybe using type this way should make it into the documentation? An example in animate()?

MilesMcBain avatar Aug 05 '19 11:08 MilesMcBain

Yeah, the type argument is def the way to go - I also think that is how most people use Cairo in general (I could be wrong).

I’m pretty sure I’m not going to accept this PR. Searching for functions passed as strings is really not a good idea IMO. I’ll have to think about a good way to allow arbitrary devices though

thomasp85 avatar Aug 05 '19 11:08 thomasp85

Maybe you're right about type, I'm still new to the pain of R on Windows.

When I said I think this PR still stands I meant the concept. The implementation has issues for sure.

Could we not just pass a function? If you can create a function that would return the 'current' device for which you use a string now, I think it could work.

MilesMcBain avatar Aug 06 '19 00:08 MilesMcBain