bitbank2 / AnimatedGIF

An optimized GIF decoder suitable for microcontrollers and PCs
Apache License 2.0
359 stars 50 forks source link

Disposal Isn’t Handled Correctly But Maybe That’s Okay #3

Closed PaintYourDragon closed 7 months ago

PaintYourDragon commented 4 years ago

Digging into why that one Krampus animation was leaving behind pixel filth, learned some stuff about GIF disposal along the way, and that dealing with it correctly while processing line-at-a-time might be so horrible that the best solution might just be “it’s not 100% compatible with all GIFs” and leave it at that.

krampus-anim

Also managed to produce a second GIF exhibiting the issue.

animgif

What these two have in common is they’re transparent animated GIFs. The background is reset to “transparent” after each frame…and that gets into Disposal Method 3 levels of impractical.

transparent-bg-1 transparent-bg-2

Okay. But…even if an opaque background color were forced, the DM2 implementation isn’t quite right. It doesn’t manifest on most GIFs because most save/convert/optimize programs refresh the whole bounding rect to paint over old and issue new graphics each frame. Producing a truly optimal DM2 GIF is a super fussy thing that most programs miss but I managed to find a non-transparent example in the ImageMagick documentation (compare this in-browser and with the GIF code):

optframe_bgnd

(The blue rects left behind in-browser are intentional, a product of the disposal method and background color. The GIF decoder leaves each new bitmap behind, not disposing them.)

The issue, mentioned in passing deep in the ImageMagick docs, is that the disposal area is supposed to be cleared AFTER the current frame…but, as written, it’s disposing the new area first. It’s usually, but not always, mostly or entirely overlapping the same area, so it usually mostly looks correct.

https://www.imagemagick.org/Usage/anim_basics/#dispose

Doing a True and Proper Disposal™ would first require keeping track of the prior frame's disposal bounds independent of the current frame…and then, disposing the old rect while drawing the new rect in the same pass. It gets into a LOT of ugly cases…

dispose-a

I mean, it’s certainly possible as a scanline algorithm, it just hurts my brain trying to work through an efficient implementation of all the various cases, when some days I can’t even remember why I went into the kitchen…

dispose-b

Hence maybe it’s just left as a “known issue” and closed, since these GIFs seem relatively rare, and some could probably be processed with a pass through ImageMagick or Photoshop or something to do the full-rect repaint, though the resulting GIF will be a little bigger.

PaintYourDragon commented 4 years ago

Also helpful in troubleshooting this:

bitbank2 commented 4 years ago

Yes, being able to manage all possible options is not possible in a low-memory implementation. Fixing the revert-to-transparent option is possible in the current code, but will take a little more effort. My first idea would be to calculate non-overlapping parts and fix them when scanline 0 is being drawn. This would also require adding the old size+position to the draw structure to know where they don't overlap. A nice to-do for a future day 😄

PaintYourDragon commented 4 years ago

Thinking about it a bit…the hard part is coalescing an opaque disposal rect that may (or not) partly overlap a new drawn rect with transparent pixels (that is, some transparent pixels might actually need to be set to the disposal color) without a ton of logic.

Here's an idea, this is just messy pseudocode…

Add a scanline bitmask to GIFIMAGE class: ((MAX_WIDTH + 7) / 8) bytes -> 320 pixels = 40 bytes RAM Add a dispose rect that's distinct from the new frame's draw rect (a few bytes)

Each frame, in GIFMakePels()…

if((dispose == 0) || (dispose color is transparent)) {
  // No disposal, just consider new draw rect:
  y_top = draw rect top;
  y_bottom = draw rect bottom;
  set dispose rect to nonsensical size, maybe height = 0 or bottom < top
} else {
  // Gonna dispose, get actual min/max extents of both rects:
  y_top = min(dispose rect top, draw rect top);
  y_bottom = max(dispose rect bottom, draw rect bottom);
}

for(y=y_top; y<=y_bottom; y++) { // Each scanline...
  do_dispose = (y overlaps dispose rect); // will be false if nonsensical size above
  do_draw = (y overlaps draw rect);
  if(do_dispose || do_draw) { // skip scanlines if dispose & draw are vertically separate
    clear scanline bitmask
    if(y overlaps dispose rect) {
      fill scanline pixel buf with dispose color from dispose rect left to dispose rect right
      fill bitmask from dispose rect left to dispose rect right
    } // end dispose
    if(y overlaps draw rect) {
      draw any/all opaque pixels into corresponding positions in scanline pixel buf
      set corresponding bits in bitmask
    } // end draw
    callback(); // Callback writes pixel buf using bitmask to coalesce addr windows
  } // end overlap dispose or draw
} // end scanline
dispose rect = draw rect;

Scanline callback would end up somewhat simpler because it doesn't need to handle any dispose cases or opaque/transparent cases as special things, just looks at the bitmask to decide how draw spans are coalesced, boom done. The heavy lifting is done in the lib and doesn’t need to be visibly inflicted on every Arduino sketch.

PaintYourDragon commented 4 years ago

The above would also make clipping a bit easier, if someone throws a GIF at it that’s larger than the screen (which might happen frequently if the display device were, say, an LED matrix). Decoder would still process the full frame, but callback would have an easier time rendering just the relevant bits.

tobozo commented 4 years ago

This GIF exceeds both in width and height the ILI9341 TFT I'm using (320x240) and seems to have a disposal problem too

macca

kamalmostafa commented 3 years ago

gifsicle is another good tool for identifying GIFs which contain the dreaded Type 3 Disposal Method frames, and can "convert them out" too.

bitbank2 commented 7 months ago

Now that I'm re-optimizing the code, I took a look at this issue again. The main challenge to get this working on MCUs is that the disposal method can affect pixels outside of the current frame. For performance reasons, only the current frame (not the whole canvas) are sent line-by-line to the GIFDraw callback to reduce the amount of data sent to SPI LCDs. The full implementation of disposal methods would need to be handled by code outside of the AnimatedGIF library and have access to at least 1 whole rendered frame buffer.