Re: [AD] Not freeing resources of non-preserved bitmaps on Android is considered harmful :)

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


22.10.2015 22:10, Trent Gamblin пишет:
2) Make current context null during suspend (via call to
eglMakeCurrent(m_display,EGL_NO_SURFACE,EGL_NO_SURFACE,EGL_NO_CONTEXT)),
but before that, save current context ID. When app is resumed, use saved
context id as a share_context parameter to eglCreateContext. Then, we will
have access to all resources of the previous context. This is the method
used by SDL and at my workplace.
In my opinion this seems like an ugly hack, like you say, it looks leaky... If
a game is suspended and resumed 100 times, you have 100 contexts...
After a bit more thinking, I decided to ask more knowledgeable people than me about this. I'll rely here what they say, if anything.
OK so the reason the NO_PRESERVE texture were not deleted was that upon resume, there is nothing to recreate the texture with. Other bitmaps' OpenGL texture is deleted (glDeleteTextures) and FBO is removed... FBO is automatically recreated by Allegro but OpenGL texture is not. A few lines below this deletion in the resume call, bitmaps are uploaded if they are not NO_PRESERVE... this recreate the OpenGL texture. So what's needed is to add another branch there that creates the OpenGL texture (it can be garbage but it needs to be created...) That's my interpretation of it from a quick look.
Hm. That might work. My patch moves the burden of recreating the texture to user, which is probably what he deserves, by using NO_PRESERVE flag, but it may be less error-prone to re-create the texture inside Allegro. I've attached a patch that removes NO_PRESERVE checks from both halt and resume functions, and adds a null pointer check in upload_bitmap_memory. Not sure about the last one - maybe we should just call ogl_create_bitmap instead?
 src/opengl/ogl_bitmap.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/opengl/ogl_bitmap.c b/src/opengl/ogl_bitmap.c
index e26f1a2..334d144 100644
--- a/src/opengl/ogl_bitmap.c
+++ b/src/opengl/ogl_bitmap.c
@@ -1056,27 +1056,28 @@ void _al_ogl_upload_bitmap_memory(ALLEGRO_BITMAP *bitmap, int format, void *ptr)
    uint8_t *dst;
    uint8_t *src;
 
-   ASSERT(ptr);
    ASSERT(al_get_current_display() == _al_get_bitmap_display(bitmap));
 
    tmp = _al_create_bitmap_params(_al_get_bitmap_display(bitmap), w, h, format,
       al_get_bitmap_flags(bitmap));
    ASSERT(tmp);
 
-   lr = al_lock_bitmap(tmp, format, ALLEGRO_LOCK_WRITEONLY);
-   ASSERT(lr);
+   if(ptr){
+      lr = al_lock_bitmap(tmp, format, ALLEGRO_LOCK_WRITEONLY);
+      ASSERT(lr);
 
-   dst = (uint8_t *)lr->data;
-   // we need to flip it
-   src = ((uint8_t *)ptr) + (pixsize * w * (h-1));
+      dst = (uint8_t *)lr->data;
+      // we need to flip it
+      src = ((uint8_t *)ptr) + (pixsize * w * (h-1));
 
-   for (y = 0; y < h; y++) {
-      memcpy(dst, src, pixsize * w);
-      dst += lr->pitch;
-      src -= pixsize * w; // minus because it's flipped
-   }
+      for (y = 0; y < h; y++) {
+         memcpy(dst, src, pixsize * w);
+         dst += lr->pitch;
+         src -= pixsize * w; // minus because it's flipped
+      }
 
-   al_unlock_bitmap(tmp);
+      al_unlock_bitmap(tmp);
+   }
 
    ((ALLEGRO_BITMAP_EXTRA_OPENGL *)bitmap->extra)->texture =
       ((ALLEGRO_BITMAP_EXTRA_OPENGL *)tmp->extra)->texture;


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/