llvmpipe: support pipe_resource-based constant buffers

Before this we only supported user-based constant buffers.

First, we basically plumb pipe_constant_buffer objects through llvmpipe
rather than pipe_resource objects.

Second, update llvmpipe_set_constant_buffer() and try_update_scene_state()
so they understand both resource- and user-based constant buffers.

The problem with user constant buffers is the potential for use-after-free,
as seen in some WebGL tests.  The next patch will flip the switch for
resource-based const buffers.

Reviewed-by: Jose Fonseca <[email protected]>

7 files changed, 48 insertions, 34 deletions

diff --git a/src/gallium/drivers/llvmpipe/lp_context.c b/src/gallium/drivers/llvmpipe/lp_context.c
index e59ae237efd..eb454b1c41f 100644
--- a/src/gallium/drivers/llvmpipe/lp_context.c
+++ b/src/gallium/drivers/llvmpipe/lp_context.c
@@ -83,7 +83,7 @@ static void llvmpipe_destroy( struct pipe_context *pipe )
 
    for (i = 0; i < Elements(llvmpipe->constants); i++) {
       for (j = 0; j < Elements(llvmpipe->constants[i]); j++) {
-         pipe_resource_reference(&llvmpipe->constants[i][j], NULL);
+         pipe_resource_reference(&llvmpipe->constants[i][j].buffer, NULL);
       }
    }
 
diff --git a/src/gallium/drivers/llvmpipe/lp_context.h b/src/gallium/drivers/llvmpipe/lp_context.h
index 5afa4360aae..b11a3d838e3 100644
--- a/src/gallium/drivers/llvmpipe/lp_context.h
+++ b/src/gallium/drivers/llvmpipe/lp_context.h
@@ -72,7 +72,7 @@ struct llvmpipe_context {
    struct pipe_blend_color blend_color;
    struct pipe_stencil_ref stencil_ref;
    struct pipe_clip_state clip;
-   struct pipe_resource *constants[PIPE_SHADER_TYPES][LP_MAX_TGSI_CONST_BUFFERS];
+   struct pipe_constant_buffer constants[PIPE_SHADER_TYPES][LP_MAX_TGSI_CONST_BUFFERS];
    struct pipe_framebuffer_state framebuffer;
    struct pipe_poly_stipple poly_stipple;
    struct pipe_scissor_state scissor;
diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
index 7d40d8cf661..5aba7a25342 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -554,7 +554,7 @@ lp_setup_set_fs_variant( struct lp_setup_context *setup,
 void
 lp_setup_set_fs_constants(struct lp_setup_context *setup,
                           unsigned num,
-                          struct pipe_resource **buffers)
+                          struct pipe_constant_buffer *buffers)
 {
    unsigned i;
 
@@ -563,11 +563,12 @@ lp_setup_set_fs_constants(struct lp_setup_context *setup,
    assert(num <= Elements(setup->constants));
 
    for (i = 0; i < num; ++i) {
-      if (setup->constants[i].current != buffers[i]) {
-         pipe_resource_reference(&setup->constants[i].current, buffers[i]);
-         setup->dirty |= LP_SETUP_NEW_CONSTANTS;
-      }
+      util_copy_constant_buffer(&setup->constants[i].current, &buffers[i]);
+   }
+   for (; i < Elements(setup->constants); i++) {
+      util_copy_constant_buffer(&setup->constants[i].current, NULL);
    }
+   setup->dirty |= LP_SETUP_NEW_CONSTANTS;
 }
 
 
@@ -873,11 +874,21 @@ try_update_scene_state( struct lp_setup_context *setup )
 
    if (setup->dirty & LP_SETUP_NEW_CONSTANTS) {
       for (i = 0; i < Elements(setup->constants); ++i) {
-         struct pipe_resource *buffer = setup->constants[i].current;
+         struct pipe_resource *buffer = setup->constants[i].current.buffer;
+         const unsigned current_size = setup->constants[i].current.buffer_size;
+         const ubyte *current_data = NULL;
 
          if (buffer) {
-            unsigned current_size = buffer->width0;
-            const void *current_data = llvmpipe_resource_data(buffer);
+            /* resource buffer */
+            current_data = (ubyte *) llvmpipe_resource_data(buffer);
+         }
+         else if (setup->constants[i].current.user_buffer) {
+            /* user-space buffer */
+            current_data = (ubyte *) setup->constants[i].current.user_buffer;
+         }
+
+         if (current_data) {
+            current_data += setup->constants[i].current.buffer_offset;
 
             /* TODO: copy only the actually used constants? */
 
@@ -1054,7 +1065,7 @@ lp_setup_destroy( struct lp_setup_context *setup )
    }
 
    for (i = 0; i < Elements(setup->constants); i++) {
-      pipe_resource_reference(&setup->constants[i].current, NULL);
+      pipe_resource_reference(&setup->constants[i].current.buffer, NULL);
    }
 
    /* free the scenes in the 'empty' queue */
diff --git a/src/gallium/drivers/llvmpipe/lp_setup.h b/src/gallium/drivers/llvmpipe/lp_setup.h
index 55fbd9be8b8..55b710dd2b4 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup.h
+++ b/src/gallium/drivers/llvmpipe/lp_setup.h
@@ -101,8 +101,7 @@ lp_setup_set_fs_variant( struct lp_setup_context *setup,
 void
 lp_setup_set_fs_constants(struct lp_setup_context *setup,
                           unsigned num,
-                          struct pipe_resource **buffers);
-
+                          struct pipe_constant_buffer *buffers);
 
 void
 lp_setup_set_alpha_ref_value( struct lp_setup_context *setup,
diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h
index 60809db50a2..4d20dd38461 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup_context.h
+++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h
@@ -128,7 +128,7 @@ struct lp_setup_context
 
    /** fragment shader constants */
    struct {
-      struct pipe_resource *current;
+      struct pipe_constant_buffer current;
       unsigned stored_size;
       const void *stored_data;
    } constants[LP_MAX_TGSI_CONST_BUFFERS];
diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c
index dced5d25a7f..bf59a43f49a 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
@@ -2408,32 +2408,32 @@ llvmpipe_set_constant_buffer(struct pipe_context *pipe,
 {
    struct llvmpipe_context *llvmpipe = llvmpipe_context(pipe);
    struct pipe_resource *constants = cb ? cb->buffer : NULL;
-   unsigned size;
-   const void *data;
-
-   if (cb && cb->user_buffer) {
-      constants = llvmpipe_user_buffer_create(pipe->screen,
-                                              (void *) cb->user_buffer,
-                                              cb->buffer_size,
-                                              PIPE_BIND_CONSTANT_BUFFER);
-   }
-
-   size = constants ? constants->width0 : 0;
-   data = constants ? llvmpipe_resource_data(constants) : NULL;
 
    assert(shader < PIPE_SHADER_TYPES);
    assert(index < Elements(llvmpipe->constants[shader]));
 
-   if(llvmpipe->constants[shader][index] == constants)
-      return;
+   /* note: reference counting */
+   util_copy_constant_buffer(&llvmpipe->constants[shader][index], cb);
 
-   draw_flush(llvmpipe->draw);
+   if (shader == PIPE_SHADER_VERTEX ||
+       shader == PIPE_SHADER_GEOMETRY) {
+      /* Pass the constants to the 'draw' module */
+      const unsigned size = cb ? cb->buffer_size : 0;
+      const ubyte *data;
 
-   /* note: reference counting */
-   pipe_resource_reference(&llvmpipe->constants[shader][index], constants);
+      if (constants) {
+         data = (ubyte *) llvmpipe_resource_data(constants);
+      }
+      else if (cb && cb->user_buffer) {
+         data = (ubyte *) cb->user_buffer;
+      }
+      else {
+         data = NULL;
+      }
+
+      if (data)
+         data += cb->buffer_offset;
 
-   if(shader == PIPE_SHADER_VERTEX ||
-      shader == PIPE_SHADER_GEOMETRY) {
       draw_set_mapped_constant_buffer(llvmpipe->draw, shader,
                                       index, data, size);
    }
diff --git a/src/gallium/drivers/llvmpipe/lp_texture.c b/src/gallium/drivers/llvmpipe/lp_texture.c
index f4d2cb6c5d0..31ea7dca50e 100644
--- a/src/gallium/drivers/llvmpipe/lp_texture.c
+++ b/src/gallium/drivers/llvmpipe/lp_texture.c
@@ -669,8 +669,12 @@ llvmpipe_transfer_map( struct pipe_context *pipe,
       }
    }
 
-   if (resource == llvmpipe->constants[PIPE_SHADER_FRAGMENT][0])
+   /* Check if we're mapping the current constant buffer */
+   if ((usage & PIPE_TRANSFER_WRITE) &&
+       resource == llvmpipe->constants[PIPE_SHADER_FRAGMENT][0].buffer) {
+      /* constants may have changed */
       llvmpipe->dirty |= LP_NEW_CONSTANTS;
+   }
 
    lpt = CALLOC_STRUCT(llvmpipe_transfer);
    if (!lpt)