Mesa (master): gallium/osmesa: Remove the broken buffer-reuse scheme.
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Sat Dec 5 00:20:43 UTC 2020
Module: Mesa
Branch: master
Commit: 26c6f6cfbb6d489cf6f995718e3e959b9fa5b233
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=26c6f6cfbb6d489cf6f995718e3e959b9fa5b233
Author: Eric Anholt <eric at anholt.net>
Date: Wed Dec 2 14:48:34 2020 -0800
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 at redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7886>
---
src/gallium/frontends/osmesa/osmesa.c | 68 +++++++-----------------------
src/gallium/targets/osmesa/test-render.cpp | 51 ++++++++++++++++++++++
2 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/src/gallium/frontends/osmesa/osmesa.c b/src/gallium/frontends/osmesa/osmesa.c
index eae487c5c03..19d70784536 100644
--- a/src/gallium/frontends/osmesa/osmesa.c
+++ b/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;
diff --git a/src/gallium/targets/osmesa/test-render.cpp b/src/gallium/targets/osmesa/test-render.cpp
index 1a720f0cd0e..39638f60cfc 100644
--- a/src/gallium/targets/osmesa/test-render.cpp
+++ b/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));
+}
More information about the mesa-commit
mailing list