android-gif-drawable icon indicating copy to clipboard operation
android-gif-drawable copied to clipboard

Implement dirty region redraws

Open frmz opened this issue 8 years ago • 20 comments

This is related to #279 but i think it's better to have a separate request since it might be helpful outside OpenGL. So, GIF decoding does not redraw the entire image but only a small part of it most of the time and Android views allow to call invalidate() on a specific Rect instead of asking for a full view redraw.

So, having the last dirty region in GifInfo object somehow (so after DDGifSlurp is called) would be really really useful especially when you have huge GIFs with a very small region being updated (which is very common actually). This should improve performance a LOT in both OpenGL and HW accelerated Canvas.

frmz avatar Apr 26 '16 10:04 frmz

Current dirty region is just bounds of current frame (defined GifImageDesc) it can be already retrieved from GifInfo.

I've looked up how it can be implemented:

  • Drawable has getDirtyBounds() method. So if it is implemented (and will return aforementioned bounds) feature should just work on API 21+. On older api levels custom Drawable.Callback can be used to achieve the same effect.
  • GifTextureView uses ANativeWindow_lock which accepts inOutDirtyBounds parameter.
  • I'm not sure what is the best way how to do it in OpenGL. glTexSubImage2D() accepts pointer to pixels in dirty region but we currently have only pointer to all pixels. Do you have any ideas?

koral-- avatar Apr 28 '16 18:04 koral--

I think the only way is to call glTexSubImage2D once every dirty line giving the proper offset in the pointer to the data OR (easier) just draw the lines that are inside the dirty areas (so always using 0 as x offset, full width and only playing with y offset and height). Doing this you can just "offset" the data pointer to the correct row and draw N lines from there.

frmz avatar Apr 28 '16 20:04 frmz

@frmz calling glTexSubImage2D() line by line several hundred times is much slower than calling it once with full width and height (eg. ~15 ms vs ~900 ms with GIF ~600x600 px and frame ~300x300 px).

Solution may be to add parameter to drawNextBitmap() which decides whether bm points to all pixels or to single frame and modify functions called there appropriately. In that case internal framebuffer will contain only the most recent frame not whole canvas (except first frame). That has also other consequences eg. when OpenGL context is lost, rest of the canvas will be also lost.

koral-- avatar May 02 '16 19:05 koral--

@koral-- i did some benchmarking, so, on a typical GL scenario we have 75% of CPU cycles doing decoding and 25% used by glTexSubImage2D(), so, i think your idea sounds great BUT it would also be awesome at that point to have an option to use a separate FB for each frame so we can cache the whole animation like most web browsers engines do, so i would do something like this

  • Constructor will have an option like "cacheAnimation" that will decide weather or not we will use an FB for each frame
  • drawNextBitmap with "drawOnlyDirtyRegion" parameter which decides weather or not the FB will contain the entire frame (if cacheAnimation is on a new FB will be allocated at each frame, i think it would be safe to make this true by default if cacheAnimation is on since it would be extremely memory innefficient to cache the entire frames)
  • A new method to free the animation FBs after animation is stopped, or a parameter to stop() call (so we can free up animation stuff when animation is stopped maybe rendering current frame in the main FB and then freeing up the others)

Doing this way a huge movie (es a full screen one) with small changing parts (like this) will draw with basically 0 cost after first loop making things extremely most efficient.

Not sure how complex this is but this would save a LOT of CPU (-> battery) in a lot of scenarios (including the drawables especially when baked by HW canvas, not only the GL implementation)

frmz avatar May 03 '16 08:05 frmz

LGTM, I'll try to implement that.

koral-- avatar May 04 '16 02:05 koral--

Great, this would open a huge amount of usage scenarios for this library

frmz avatar May 04 '16 09:05 frmz

Hi @koral-- is this feature still under review? In my app most use cases have a GIF that keeps looping so having the decode run over and over is a battery hog at the moment

frmz avatar Jul 19 '16 17:07 frmz

@frmz sorry for the delay. It is already reviewed but I have no time to implement it yet. I'll try to take care of it at the end of this week but can't promise any ETA.

koral-- avatar Jul 19 '16 18:07 koral--

OK, work has finally started. There is a dirty-regions branch for this feature, sample project repo is also updated. Currently only caching entire frames is implemented but I'll push updates soon.

koral-- avatar Aug 14 '16 19:08 koral--

Awesome! Will test asap

frmz avatar Aug 14 '16 19:08 frmz

Next update pushed: framebuffers consist of only dirty regions. TODOs:

  • [ ] preparing canvas before drawing 1st frame (drawing background)
  • [ ] frame disposal
  • [ ] GL objects disposal
  1. can be achieved either by having special case for 1st frame (apart from its own pixels it will contain background color and will be drawn without offset) or (probably better) use some GL APIs to fill texture of 1st frame with background color or use separate texture for background.

It seems that after all frames are buffered some resources can be freed eg. input source (file descriptor), framebuffer used to transfer pixels from client space to GL, raster buffer etc.

There is also one another consequence of saving frames. Theoretically if modifiable source is used (eg. file) it can be altered during animation. Without frame caching those changes would be reflected. This is limited because metadata is only read once so it is rather hard to use this feature intentionally and this info can be treated as a bit of gossip.

koral-- avatar Sep 12 '16 02:09 koral--

@koral-- sorry for not being able to test this in the last months, i had to park this for a while, anyway, i have tested this branch on different setups and i am not seeing the performance difference i was expecting, do you see the same? i have tested by enabling and disabling the last param in the "getBitmap" call, inside the "slurp" function and on the profiler CPU usage is nearly identical, even on GIF where only a small part of the screen updates, for a full screen GIF on a Nexus 6P CPU usage is around 30% which is quite too much

frmz avatar Jan 24 '17 13:01 frmz

I haven't done much more than commits linked above. Sorry about slowness. AFAIK utilization of that param is almost not implemented yet. However frame buffering to textures should work but you have to manually advance frames. As you can see on diff: https://github.com/koral--/android-gif-drawable/compare/dirty-regions#diff-17a3b18c4db873c214b51cb0bac315b2R184 if frame was rendered once to framebuffer its texture will be just bound.

koral-- avatar Jan 26 '17 00:01 koral--

How complex would it be to store changes on non "key" frames in dedicated FBs? This would allow to potentially load the GIF entirely in memory, if the changes are not a lot it might not be that memory intensive. This means allocating a properly sized FB for each render pass but allocate only the dirty region size after the first / non key frame.

frmz avatar Jan 27 '17 10:01 frmz

Currently (in terms of OpenGL) there is one additional frame buffer for entire GIF and one texture for each frame. Each texture has the same size equal to canvas size, but only needed area is modified.

Do you mean separate OpenGL frame buffer for each frame? Or only that textures should have size equal to corresponding frame size? Or maybe something else?

Another, partially independent change which may improve performance is to remove client's address space frame buffers (those allocated by malloc/calloc) and pixel transfers (glTex[Sub]Image2D) and use glMapBufferRange so functions setting pixels will work directly on mapped GL memory.

koral-- avatar Jan 29 '17 18:01 koral--

I meant an OpenGL frame buffer for each frame with a dynamic size depending on the area being modified, so if we are modifying small areas we can potentially cache the entire animation in memory.

Agree on glMapBufferRange, but i think on Android its quite tricky to make that work.

frmz avatar Jan 30 '17 09:01 frmz

OK, so we create OpenGL frame buffer for each frame. Each of them has own GL texture (associated via glFramebufferTexture2D) which contains only pixels of given frame, right?

How do you want to display content of those framebuffers at correct offset?

koral-- avatar Feb 01 '17 20:02 koral--

Good point, maybe you could use an array that maps small frame buffer lines to big frame buffer line offsets since afaik GIF works on a "line" basis anyway or you can just use rect dirty regions and store the top left corner offset.

frmz avatar Feb 02 '17 15:02 frmz

OK, will try. It seems that frame offset can be achieved using matrix translation like in current sample: https://github.com/koral--/android-gif-drawable/blob/master/sample/src/main/java/pl/droidsonroids/gif/sample/GifTexImage2DFragment.java#L152 Matrix can be changed in onDrawFrame so each GIF frame can have own translation.

koral-- avatar Feb 08 '17 01:02 koral--

Yeah if the dirty region is always a rect area it should be trivial

frmz avatar Feb 08 '17 14:02 frmz