gallium/osmesa: Remove the broken buffer-reuse scheme.

Besides leaking and a lack of thread-safety, it would also incorrectly
share front buffers if multiple contexts happened to use the same
size/format, as demonstrated by the new unit test.

Closes: #2035
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7886>

src/gallium/frontends/osmesa/osmesa.c

@@ -121,18 +121,6 @@ struct osmesa_context
    struct pp_queue_t *pp;
 };
 
-
-/**
- * Linked list of all osmesa_buffers.
- * We can re-use an osmesa_buffer from one OSMesaMakeCurrent() call to
- * the next unless the color/depth/stencil/accum formats change.
- * We have to do this to be compatible with the original OSMesa implementation
- * because some apps call OSMesaMakeCurrent() several times during rendering
- * a frame.
- */
-static struct osmesa_buffer *BufferList = NULL;
-
-
 /**
  * Called from the ST manager.
  */
@@ -503,41 +491,12 @@ osmesa_create_buffer(enum pipe_format color_format,
 
       osmesa_init_st_visual(&osbuffer->visual, color_format,
                             ds_format, accum_format);
-
-      /* insert into linked list */
-      osbuffer->next = BufferList;
-      BufferList = osbuffer;
    }
 
    return osbuffer;
 }
 
 
-/**
- * Search linked list for a buffer with matching pixel formats and size.
- */
-static struct osmesa_buffer *
-osmesa_find_buffer(enum pipe_format color_format,
-                   enum pipe_format ds_format,
-                   enum pipe_format accum_format,
-                   GLsizei width, GLsizei height)
-{
-   struct osmesa_buffer *b;
-
-   /* Check if we already have a suitable buffer for the given formats */
-   for (b = BufferList; b; b = b->next) {
-      if (b->visual.color_format == color_format &&
-          b->visual.depth_stencil_format == ds_format &&
-          b->visual.accum_format == accum_format &&
-          b->width == width &&
-          b->height == height) {
-         return b;
-      }
-   }
-   return NULL;
-}
-
-
 static void
 osmesa_destroy_buffer(struct osmesa_buffer *osbuffer)
 {
@@ -786,7 +745,6 @@ OSMesaMakeCurrent(OSMesaContext osmesa, void *buffer, GLenum type,
                   GLsizei width, GLsizei height)
 {
    struct st_api *stapi = get_st_api();
-   struct osmesa_buffer *osbuffer;
    enum pipe_format color_format;
 
    if (!osmesa && !buffer) {
@@ -805,28 +763,34 @@ OSMesaMakeCurrent(OSMesaContext osmesa, void *buffer, GLenum type,
    }
 
    /* See if we already have a buffer that uses these pixel formats */
-   osbuffer = osmesa_find_buffer(color_format,
-                                 osmesa->depth_stencil_format,
-                                 osmesa->accum_format, width, height);
-   if (!osbuffer) {
-      /* Existing buffer found, create new buffer */
-      osbuffer = osmesa_create_buffer(color_format,
+   if (osmesa->current_buffer &&
+       (osmesa->current_buffer->visual.color_format != color_format ||
+        osmesa->current_buffer->visual.depth_stencil_format != osmesa->depth_stencil_format ||
+        osmesa->current_buffer->visual.accum_format != osmesa->accum_format)) {
+      osmesa_destroy_buffer(osmesa->current_buffer);
+   }
+
+   if (!osmesa->current_buffer) {
+      osmesa->current_buffer = osmesa_create_buffer(color_format,
                                       osmesa->depth_stencil_format,
                                       osmesa->accum_format);
    }
 
+   struct osmesa_buffer *osbuffer = osmesa->current_buffer;
+
    osbuffer->width = width;
    osbuffer->height = height;
    osbuffer->map = buffer;
 
-   /* XXX unused for now */
-   (void) osmesa_destroy_buffer;
-
-   osmesa->current_buffer = osbuffer;
    osmesa->type = type;
 
    stapi->make_current(stapi, osmesa->stctx, osbuffer->stfb, osbuffer->stfb);
 
+   /* XXX: We should probably load the current color value into the buffer here
+    * to match classic swrast behavior (context's fb starts with the contents of
+    * your pixel buffer).
+    */
+
    if (!osmesa->ever_used) {
       /* one-time init, just postprocessing for now */
       boolean any_pp_enabled = FALSE;

src/gallium/targets/osmesa/test-render.cpp

@@ -215,3 +215,54 @@ TEST(OSMesaRenderTest, depth)
    EXPECT_EQ(depth[w * 1 + 0], 0x00ffffff);
    EXPECT_EQ(depth[w * 1 + 1], 0x00000000);
 }
+
+static uint32_t be_bswap32(uint32_t x)
+{
+   if (UTIL_ARCH_BIG_ENDIAN)
+      return util_bswap32(x);
+   else
+      return x;
+}
+
+TEST(OSMesaRenderTest, separate_buffers_per_context)
+{
+   std::unique_ptr<osmesa_context, decltype(&OSMesaDestroyContext)> ctx1{
+      OSMesaCreateContext(GL_RGBA, NULL), &OSMesaDestroyContext};
+   std::unique_ptr<osmesa_context, decltype(&OSMesaDestroyContext)> ctx2{
+      OSMesaCreateContext(GL_RGBA, NULL), &OSMesaDestroyContext};
+   ASSERT_TRUE(ctx1);
+   ASSERT_TRUE(ctx2);
+
+   uint32_t pixel1, pixel2;
+
+   ASSERT_EQ(OSMesaMakeCurrent(ctx1.get(), &pixel1, GL_UNSIGNED_BYTE, 1, 1), GL_TRUE);
+   glClearColor(1.0, 0.0, 0.0, 0.0);
+   glClear(GL_COLOR_BUFFER_BIT);
+   glFinish();
+   EXPECT_EQ(pixel1, be_bswap32(0x000000ff));
+
+   ASSERT_EQ(OSMesaMakeCurrent(ctx2.get(), &pixel2, GL_UNSIGNED_BYTE, 1, 1), GL_TRUE);
+   glClearColor(0.0, 1.0, 0.0, 0.0);
+   glClear(GL_COLOR_BUFFER_BIT);
+   glFinish();
+   EXPECT_EQ(pixel1, be_bswap32(0x000000ff));
+   EXPECT_EQ(pixel2, be_bswap32(0x0000ff00));
+
+   /* Leave a dangling render to pixel2 as we switch contexts (there should be
+    */
+   glClearColor(0.0, 0.0, 1.0, 0.0);
+   glClear(GL_COLOR_BUFFER_BIT);
+
+   ASSERT_EQ(OSMesaMakeCurrent(ctx1.get(), &pixel1, GL_UNSIGNED_BYTE, 1, 1), GL_TRUE);
+   /* Draw something off screen to trigger a real flush.  We should have the
+    * same contents in pixel1 as before
+    */
+   glBegin(GL_TRIANGLES);
+   glVertex2f(-2, -2);
+   glVertex2f(-2, -2);
+   glVertex2f(-2, -2);
+   glEnd();
+   glFinish();
+   EXPECT_EQ(pixel1, be_bswap32(0x000000ff));
+   EXPECT_EQ(pixel2, be_bswap32(0x00ff0000));
+}