yihui / animation

A gallery of animations in statistics and utilities to create animations
https://yihui.org/animation/
206 stars 60 forks source link

Fix `loop = FALSE` behaviour #89

Closed ottlngr closed 7 years ago

ottlngr commented 7 years ago

Setting loop = FALSE in ani.options does not work, as it forwards -loop FALSE instead of the desired -loop 1 to ImageMagick. This PR fixes this issue and allows ani.options to work as supposed in the documentation.

Bug mistakenly documented in gganimate: https://github.com/dgrtwo/gganimate/issues/18

yihui commented 7 years ago

I think we need to fix the documentation. I believe loop is supposed to be either TRUE or a number (the number of times). TRUE basically implies loop = 0.

ottlngr commented 7 years ago

Yes, that's the other possibility. But to my way of thinking, one should not mix up boolean and numeric values for arguments. On the other hand this would break existing code.

If desired, I can fix the documentation and file a PR, instead of changing code.

yulijia commented 7 years ago

@ottlngr

I think we misunderstand the loop meaning in ImageMagick.

The document in ImageMagick , http://www.imagemagick.org/Usage/anim_basics/

-loop {number}
Number of times the GIF animation is to cycle though the image sequence before stopping. It is an output 'image write' setting, so can be set anywhere on the command line, though only the last such setting will be used. Usually this set by default, to zero (infinite loop), however if any image read in has a different value, then this setting will be set to that images value. As such I recommend that you always set "-loop" when creating a GIF animation, after all the images has been read in.

In the code, you use 1 cycle instead of number of cycles, it definitely not what we want.

-  loop = ifelse(isTRUE(ani.options('loop')), 0, ani.options('loop'))
+  loop = ifelse(isTRUE(ani.options('loop')), 0, 1)

So, what we need is fixing -loop documentation in animation package.

loop: Number of times the GIF animation is to cycle though the image sequence before stopping. Usually this set by default, to zero or boolean value TRUE (infinite loop).

Would you mind help us fix the documentation?

Thanks.

ottlngr commented 7 years ago

https://github.com/yihui/animation/pull/90