Re: [AD] android_set_current_display

[Jon Rafkind <workmin@xxxxxxxxxx>]

There is more to the story here. The display->render_state.write_mask was not set appropriately before the opengl state was set. This is because the flow of the system is

al_create_display
android_create_display
_al_android_setup_opengl_view
al_set_target_backbuffer
al_set_target_bitmap
android_set_current_display
_al_ogl_update_render_state

And finally in _al_ogl_update_render_state is a call to glColorMask based on display->render_state.write_mask. But the write_mask is not set until after al_create_display calls android_create_display which is after that entire chain of events. Somehow android_set_current_display is not called again so the ogl state is never updated with the proper write mask.

I tried moving the call to _al_android_setup_opengl_view out of android_create_display and into android_set_current_display and it works. I don't know if this is the proper solution but heres a patch anyway.

BTW I noticed that android_acknowledge_drawing_resume could probably just call android_set_current_display instead of clear_current/make_current/setup_opengl_view.

diff --git a/src/android/android_display.c b/src/android/android_display.c
index 8ced6c4..a760334 100644
--- a/src/android/android_display.c
+++ b/src/android/android_display.c
@@ -538,8 +538,6 @@ static ALLEGRO_DISPLAY *android_create_display(int w, int h)
    _al_android_clear_current(_al_android_get_jnienv(), d);
    _al_android_make_current(_al_android_get_jnienv(), d);
   
-   _al_android_setup_opengl_view(display);
-
    /* Don't need to repeat what this does */
    android_change_display_option(display, ALLEGRO_SUPPORTED_ORIENTATIONS, al_get_new_display_option(ALLEGRO_SUPPORTED_ORIENTATIONS, NULL));
 
@@ -597,6 +595,7 @@ static bool android_set_current_display(ALLEGRO_DISPLAY *dpy)
    ALLEGRO_DEBUG("make current %p", dpy);
    if (dpy) _al_android_make_current(_al_android_get_jnienv(), (ALLEGRO_DISPLAY_ANDROID*)dpy);
 
+   _al_android_setup_opengl_view(dpy);
    _al_ogl_update_render_state(dpy);
 
    return true;


On 11/05/2012 10:38 AM, Thomas Fjellstrom wrote:
> On Mon Nov 5, 2012, Elias Pschernig wrote:
>> On Mon, Nov 5, 2012 at 9:35 AM, Jon Rafkind <workmin@xxxxxxxxxx> wrote:
>>> There is a bug in android_set_current_display where the new display will
>>> never be set properly. The short story is that it first checks if
>>> al_get_current_display() == dpy, where dpy is the argument to the
>>> function, and if they are equal then the function aborts (presumably to
>>> save time). But these two things will always be equal.
>>>
>>> With this client code:
>>>   ALLEGRO_BITMAP * old = al_get_target_bitmap();
>>>   al_set_target_bitmap(NULL);
>>>   al_set_target_bitmap(old);
>>>
>>> The old->display will be the current active display. Setting the target
>>> to NULL will unset the current display. Then setting the target back to
>>> old will try to reset the display to old->display.
>>>
>>> The logic Allegro follows is
>>>
>>> al_set_target_bitmap(old)
>>> // tls.c: al_set_target_bitmap
>>> ...
>>> if (old_display != new_display){
>>>
>>>    ...
>>>    tls->current_display = new_display;
>>>    new_display->vt->set_current_display(new_display)
>>>    
>>>    // android/android_display.c:android_set_current_display
>>>    if (al_get_current_display() == dpy) return true;
>>>
>>> Where al_get_current_display() is
>>>
>>> // tls.c:al_get_current_display
>>>
>>>  return tls->current_display;
>>>
>>> So al_get_current_display() will always be equal to the dpy parameter to
>>> android_set_current_display. By simply commenting out the check in
>>> android_set_current_display() and letting it always set the display
>>> (with make_current) my application works as expected. I guess I should
>>> mention that this bug eventually causes all opengl calls to fail because
>>> there is no context for them.
>> Other ports have no such check - so why does the Android port have it?
> 2 possibilities, the tls function was changed after the fact, or potentially 
> more likely, I was in my "CHECK ALL THE INPUTS" mode, when it wasn't wanted or 
> desired. (or for that matter, correct)
>
>> ---------------------------------------------------------------------------
>> --- LogMeIn Central: Instant, anywhere, Remote PC access and management.
>> Stay in control, update software, and manage PCs from one command center
>> Diagnose problems and improve visibility into emerging IT issues
>> Automate, monitor and manage. Do more in less time with Central
>> http://p.sf.net/sfu/logmein12331_d2d
>