Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement dirty region redraws #287

Open
frmz opened this issue Apr 26, 2016 · 20 comments
Open

Implement dirty region redraws #287

frmz opened this issue Apr 26, 2016 · 20 comments

Comments

@frmz
Copy link
Contributor

frmz commented Apr 26, 2016

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 added a commit to frmz/android-gif-drawable that referenced this issue Apr 27, 2016
@koral--
Copy link
Owner

koral-- commented Apr 28, 2016

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?

@frmz
Copy link
Contributor Author

frmz commented Apr 28, 2016

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.

@koral--
Copy link
Owner

koral-- commented May 2, 2016

@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.

@frmz
Copy link
Contributor Author

frmz commented May 3, 2016

@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)

@koral--
Copy link
Owner

koral-- commented May 4, 2016

LGTM, I'll try to implement that.

@frmz
Copy link
Contributor Author

frmz commented May 4, 2016

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

@frmz
Copy link
Contributor Author

frmz commented Jul 19, 2016

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

@koral--
Copy link
Owner

koral-- commented Jul 19, 2016

@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--
Copy link
Owner

koral-- commented Aug 14, 2016

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.

@frmz
Copy link
Contributor Author

frmz commented Aug 14, 2016

Awesome! Will test asap

koral-- added a commit that referenced this issue Sep 12, 2016
koral-- added a commit that referenced this issue Sep 12, 2016
@koral--
Copy link
Owner

koral-- commented Sep 12, 2016

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.

@frmz
Copy link
Contributor Author

frmz commented Jan 24, 2017

@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

@koral--
Copy link
Owner

koral-- commented Jan 26, 2017

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.

@frmz
Copy link
Contributor Author

frmz commented Jan 27, 2017

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.

@koral--
Copy link
Owner

koral-- commented Jan 29, 2017

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.

@frmz
Copy link
Contributor Author

frmz commented Jan 30, 2017

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.

@koral--
Copy link
Owner

koral-- commented Feb 1, 2017

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?

@frmz
Copy link
Contributor Author

frmz commented Feb 2, 2017

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.

@koral--
Copy link
Owner

koral-- commented Feb 8, 2017

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.

@frmz
Copy link
Contributor Author

frmz commented Feb 8, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants