[AD] opengl clipping state

[Peter Wang <novalazy@xxxxxxxxxx>]

exnew_lines shows a bug in the OpenGL driver: a call to
al_set_clipping_rectangle(bitmap) where bitmap is the target bitmap won't
update the OpenGL state immediately.  Does the following fix look okay?

Previously I was worried about what happens if you set the clipping rectangle
from one thread while it is set as the target bitmap for another thread, but I
guess you can't do that.

Peter

diff --git a/include/allegro5/internal/aintern_bitmap.h b/include/allegro5/internal/aintern_bitmap.h
index 4908efe..33af5c8 100644
--- a/include/allegro5/internal/aintern_bitmap.h
+++ b/include/allegro5/internal/aintern_bitmap.h
@@ -84,6 +84,8 @@ struct ALLEGRO_BITMAP_INTERFACE
     * read the contents of an associated texture.
     */
 
+   void (*update_clipping_rectangle)(ALLEGRO_BITMAP *bitmap);
+
    void (*destroy_bitmap)(ALLEGRO_BITMAP *bitmap);
 
    ALLEGRO_LOCKED_REGION * (*lock_region)(ALLEGRO_BITMAP *bitmap,
diff --git a/include/allegro5/internal/aintern_opengl.h b/include/allegro5/internal/aintern_opengl.h
index 04feca1..10261ec 100644
--- a/include/allegro5/internal/aintern_opengl.h
+++ b/include/allegro5/internal/aintern_opengl.h
@@ -87,6 +87,7 @@ ALLEGRO_BITMAP *_al_ogl_create_sub_bitmap(ALLEGRO_DISPLAY *d, ALLEGRO_BITMAP *pa
 
 /* common driver */
 void _al_ogl_set_target_bitmap(ALLEGRO_DISPLAY *display, ALLEGRO_BITMAP *bitmap);
+void _al_ogl_setup_bitmap_clipping(const ALLEGRO_BITMAP *bitmap);
 ALLEGRO_BITMAP *_al_ogl_get_backbuffer(ALLEGRO_DISPLAY *d);
 ALLEGRO_BITMAP_OGL* _al_ogl_create_backbuffer(ALLEGRO_DISPLAY *disp);
 void _al_ogl_destroy_backbuffer(ALLEGRO_BITMAP_OGL *b);

diff --git a/src/display_new.c b/src/display_new.c
index dd753cb..e8115c2 100644
--- a/src/display_new.c
+++ b/src/display_new.c
@@ -458,6 +458,7 @@ bool al_wait_for_vsync(void)
  * pixels get clipped to. The default is to clip pixels
  * to the entire bitmap.
  */
+/* XXX this seems like it belongs in bitmap_new.c */
 void al_set_clipping_rectangle(int x, int y, int width, int height)
 {
    ALLEGRO_BITMAP *bitmap = al_get_target_bitmap();
@@ -483,6 +484,10 @@ void al_set_clipping_rectangle(int x, int y, int width, int height)
    bitmap->ct = y;
    bitmap->cr = x + width;
    bitmap->cb = y + height;
+
+   if (bitmap->vt->update_clipping_rectangle) {
+      bitmap->vt->update_clipping_rectangle(bitmap);
+   }
 }
 
 
@@ -491,6 +496,7 @@ void al_set_clipping_rectangle(int x, int y, int width, int height)
  *
  * Gets the clipping rectangle of the target bitmap.
  */
+/* XXX this seems like it belongs in bitmap_new.c */
 void al_get_clipping_rectangle(int *x, int *y, int *w, int *h)
 {
    ALLEGRO_BITMAP *bitmap = al_get_target_bitmap();
diff --git a/src/opengl/ogl_bitmap.c b/src/opengl/ogl_bitmap.c
index d3b04ad..59dc2dc 100644
--- a/src/opengl/ogl_bitmap.c
+++ b/src/opengl/ogl_bitmap.c
@@ -314,6 +313,19 @@ static bool ogl_upload_bitmap(ALLEGRO_BITMAP *bitmap, int x, int y,
 
 
 
+static void ogl_update_clipping_rectangle(ALLEGRO_BITMAP *bitmap)
+{
+   ALLEGRO_DISPLAY *display = al_get_current_display();
+   ALLEGRO_DISPLAY_OGL *ogl_disp = (void *)display;
+   ALLEGRO_BITMAP_OGL *ogl_bitmap = (void *)bitmap;
+
+   if (ogl_disp->opengl_target == ogl_bitmap) {
+      _al_ogl_setup_bitmap_clipping(bitmap);
+   }
+}
+
+
+
 /* OpenGL cannot "lock" pixels, so instead we update our memory copy and
  * return a pointer into that.
  */
@@ -450,12 +462,13 @@ static ALLEGRO_BITMAP_INTERFACE *ogl_bitmap_driver(void)
    memset(glbmp_vt, 0, sizeof *glbmp_vt);
 
    glbmp_vt->draw_bitmap = ogl_draw_bitmap;
-   glbmp_vt->upload_bitmap = ogl_upload_bitmap;
-   glbmp_vt->destroy_bitmap = ogl_destroy_bitmap;
-   glbmp_vt->draw_scaled_bitmap = ogl_draw_scaled_bitmap;
    glbmp_vt->draw_bitmap_region = ogl_draw_bitmap_region;
-   glbmp_vt->draw_rotated_scaled_bitmap = ogl_draw_rotated_scaled_bitmap;
+   glbmp_vt->draw_scaled_bitmap = ogl_draw_scaled_bitmap;
    glbmp_vt->draw_rotated_bitmap = ogl_draw_rotated_bitmap;
+   glbmp_vt->draw_rotated_scaled_bitmap = ogl_draw_rotated_scaled_bitmap;
+   glbmp_vt->upload_bitmap = ogl_upload_bitmap;
+   glbmp_vt->update_clipping_rectangle = ogl_update_clipping_rectangle;
+   glbmp_vt->destroy_bitmap = ogl_destroy_bitmap;
    glbmp_vt->lock_region = ogl_lock_region;
    glbmp_vt->unlock_region = ogl_unlock_region;
 
diff --git a/src/opengl/ogl_display.c b/src/opengl/ogl_display.c
index ab89752..f499f14 100644
--- a/src/opengl/ogl_display.c
+++ b/src/opengl/ogl_display.c
@@ -80,33 +80,38 @@ void _al_ogl_set_target_bitmap(ALLEGRO_DISPLAY *display, ALLEGRO_BITMAP *bitmap)
    }
 
    if (ogl_disp->opengl_target == ogl_bitmap) {
-      int x_1, y_1, x_2, y_2;
+      _al_ogl_setup_bitmap_clipping(bitmap);
+   }
+}
 
-      x_1 = bitmap->cl;
-      y_1 = bitmap->ct;
-      /* In OpenGL, coordinates are the top-left corner of pixels, so we need
-       * to add one to the right and bottom edge.
-       */
-      x_2 = bitmap->cr + 1;
-      y_2 = bitmap->cb + 1;
-
-      /* Drawing onto the sub bitmap is handled by clipping the parent. */
-      if (bitmap->parent) {
-         x_1 += bitmap->xofs;
-         y_1 += bitmap->yofs;
-         x_2 += bitmap->xofs;
-         y_2 += bitmap->yofs;
-      }
 
-      if (x_1 == 0 &&  y_1 == 0 && x_2 == bitmap->w && y_2 == bitmap->h)
-      {
-         glDisable(GL_SCISSOR_TEST);
-      }
-      else {
-         glEnable(GL_SCISSOR_TEST);
-         /* OpenGL is upside down, so must adjust y_2 to the height. */
-         glScissor(x_1, bitmap->h - y_2, x_2 - x_1, y_2 - y_1);
-      }
+void _al_ogl_setup_bitmap_clipping(const ALLEGRO_BITMAP *bitmap)
+{
+   int x_1, y_1, x_2, y_2;
+
+   x_1 = bitmap->cl;
+   y_1 = bitmap->ct;
+   /* In OpenGL, coordinates are the top-left corner of pixels, so we need
+    * to add one to the right and bottom edge.
+    */
+   x_2 = bitmap->cr + 1;
+   y_2 = bitmap->cb + 1;
+
+   /* Drawing onto the sub bitmap is handled by clipping the parent. */
+   if (bitmap->parent) {
+      x_1 += bitmap->xofs;
+      y_1 += bitmap->yofs;
+      x_2 += bitmap->xofs;
+      y_2 += bitmap->yofs;
+   }
+
+   if (x_1 == 0 &&  y_1 == 0 && x_2 == bitmap->w && y_2 == bitmap->h) {
+      glDisable(GL_SCISSOR_TEST);
+   }
+   else {
+      glEnable(GL_SCISSOR_TEST);
+      /* OpenGL is upside down, so must adjust y_2 to the height. */
+      glScissor(x_1, bitmap->h - y_2, x_2 - x_1, y_2 - y_1);
    }
 }