[Mesa-dev] [PATCH 3/4] glx: Fix indirect multi-texture GL_DOUBLE coordinate arrays.
Ian Romanick
idr at freedesktop.org
Tue Jun 28 01:22:17 UTC 2016
This patch is
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
On 06/23/2016 11:15 AM, Matt Turner wrote:
> From: Colin McDonald <cjmmail10-bz at yahoo.co.uk>
>
> There is no draw arrays protocol support for multi-texture coordinate
> arrays, so it is implemented by sending batches of immediate mode
> commands from emit_element_none in indirect_vertex_array.c. This sends
> the target texture unit (which has been previously setup in the
> array_state header field), followed by the texture coordinates. But for
> GL_DOUBLE coordinates the texture unit must be sent *after* the texture
> coordinates. This is documented in the glx protocol description, and can
> also be seen in the indirect.c immediate mode commands generated from
> gl_API.xml. Sending the target texture unit in the wrong place can crash
> the remote X server.
>
> To fix this required some more extensive changes to
> indirect_vertex_array.c and indirect_vertex_array_priv.h, in order to
> remove the texture unit value out of the array_state "header" field, and
> send it separately.
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61907
> ---
> src/glx/indirect_vertex_array.c | 68 +++++++++++++++++++++++++-----------
> src/glx/indirect_vertex_array_priv.h | 12 ++-----
> 2 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/src/glx/indirect_vertex_array.c b/src/glx/indirect_vertex_array.c
> index 01fab33..0374093 100644
> --- a/src/glx/indirect_vertex_array.c
> +++ b/src/glx/indirect_vertex_array.c
> @@ -240,8 +240,6 @@ __glXInitVertexArrayState(struct glx_context * gc)
>
> arrays->arrays[4 + i].old_DrawArrays_possible = (i == 0);
> arrays->arrays[4 + i].index = i;
> -
> - arrays->arrays[4 + i].header[1] = i + GL_TEXTURE0;
> }
>
> i = 4 + texture_units;
> @@ -274,8 +272,6 @@ __glXInitVertexArrayState(struct glx_context * gc)
>
> arrays->arrays[idx + i].old_DrawArrays_possible = 0;
> arrays->arrays[idx + i].index = idx;
> -
> - arrays->arrays[idx + i].header[1] = idx;
> }
>
> i += vertex_program_attribs;
> @@ -325,7 +321,7 @@ calculate_single_vertex_size_none(const struct array_state_vector *arrays)
>
> for (i = 0; i < arrays->num_arrays; i++) {
> if (arrays->arrays[i].enabled) {
> - single_vertex_size += ((uint16_t *) arrays->arrays[i].header)[0];
> + single_vertex_size += arrays->arrays[i].header[0];
> }
> }
>
> @@ -353,17 +349,45 @@ emit_element_none(GLubyte * dst,
> * protocol is for a 4Nus. Since the sizes are small, the
> * performance impact on modern processors should be negligible.
> */
> - (void) memset(dst, 0, ((uint16_t *) arrays->arrays[i].header)[0]);
> -
> - (void) memcpy(dst, arrays->arrays[i].header,
> - arrays->arrays[i].header_size);
> -
> - dst += arrays->arrays[i].header_size;
> -
> - (void) memcpy(dst, ((GLubyte *) arrays->arrays[i].data) + offset,
> - arrays->arrays[i].element_size);
> -
> - dst += __GLX_PAD(arrays->arrays[i].element_size);
> + (void) memset(dst, 0, arrays->arrays[i].header[0]);
> +
> + (void) memcpy(dst, arrays->arrays[i].header, 4);
> +
> + dst += 4;
> +
> + if (arrays->arrays[i].key == GL_TEXTURE_COORD_ARRAY &&
> + arrays->arrays[i].index > 0) {
> + /* Multi-texture coordinate arrays require the texture target
> + * to be sent. For doubles it is after the data, for everything
> + * else it is before.
> + */
> + GLenum texture = arrays->arrays[i].index + GL_TEXTURE0;
> + if (arrays->arrays[i].data_type == GL_DOUBLE) {
> + (void) memcpy(dst, ((GLubyte *) arrays->arrays[i].data) + offset,
> + arrays->arrays[i].element_size);
> + dst += arrays->arrays[i].element_size;
> + (void) memcpy(dst, &texture, 4);
> + dst += 4;
> + } else {
> + (void) memcpy(dst, &texture, 4);
> + dst += 4;
> + (void) memcpy(dst, ((GLubyte *) arrays->arrays[i].data) + offset,
> + arrays->arrays[i].element_size);
> + dst += __GLX_PAD(arrays->arrays[i].element_size);
> + }
> + } else if (arrays->arrays[i].key == GL_VERTEX_ATTRIB_ARRAY_POINTER) {
> + /* Vertex attribute data requires the index sent first.
> + */
> + (void) memcpy(dst, &arrays->arrays[i].index, 4);
> + dst += 4;
> + (void) memcpy(dst, ((GLubyte *) arrays->arrays[i].data) + offset,
> + arrays->arrays[i].element_size);
> + dst += __GLX_PAD(arrays->arrays[i].element_size);
> + } else {
> + (void) memcpy(dst, ((GLubyte *) arrays->arrays[i].data) + offset,
> + arrays->arrays[i].element_size);
> + dst += __GLX_PAD(arrays->arrays[i].element_size);
> + }
> }
> }
>
> @@ -1099,6 +1123,10 @@ __indirect_glMultiDrawElementsEXT(GLenum mode, const GLsizei * count,
> }
>
>
> +/* The HDR_SIZE macro argument is the command header size (4 bytes)
> + * plus any additional index word e.g. for texture units or vertex
> + * attributes.
> + */
> #define COMMON_ARRAY_DATA_INIT(a, PTR, TYPE, STRIDE, COUNT, NORMALIZED, HDR_SIZE, OPCODE) \
> do { \
> (a)->data = PTR; \
> @@ -1111,9 +1139,8 @@ __indirect_glMultiDrawElementsEXT(GLenum mode, const GLsizei * count,
> (a)->true_stride = (STRIDE == 0) \
> ? (a)->element_size : STRIDE; \
> \
> - (a)->header_size = HDR_SIZE; \
> - ((uint16_t *) (a)->header)[0] = __GLX_PAD((a)->header_size + (a)->element_size); \
> - ((uint16_t *) (a)->header)[1] = OPCODE; \
> + (a)->header[0] = __GLX_PAD(HDR_SIZE + (a)->element_size); \
> + (a)->header[1] = OPCODE; \
> } while(0)
>
>
> @@ -1691,8 +1718,7 @@ __indirect_glVertexAttribPointer(GLuint index, GLint size,
> opcode);
>
> true_immediate_size = __glXTypeSize(type) * true_immediate_count;
> - ((uint16_t *) (a)->header)[0] = __GLX_PAD(a->header_size
> - + true_immediate_size);
> + a->header[0] = __GLX_PAD(8 + true_immediate_size);
>
> if (a->enabled) {
> arrays->array_info_cache_valid = GL_FALSE;
> diff --git a/src/glx/indirect_vertex_array_priv.h b/src/glx/indirect_vertex_array_priv.h
> index 56dac37..488dacb 100644
> --- a/src/glx/indirect_vertex_array_priv.h
> +++ b/src/glx/indirect_vertex_array_priv.h
> @@ -87,16 +87,10 @@ struct array_state
>
> /**
> * Pre-calculated GLX protocol command header.
> + * This contains two 16-bit words: the command length and the command
> + * opcode.
> */
> - uint32_t header[2];
> -
> - /**
> - * Size of the header data. For simple data, like glColorPointerfv,
> - * this is 4. For complex data that requires either a count (e.g.,
> - * glWeightfvARB), an index (e.g., glVertexAttrib1fvARB), or a
> - * selector enum (e.g., glMultiTexCoord2fv) this is 8.
> - */
> - unsigned header_size;
> + uint16_t header[2];
>
> /**
> * Set to \c GL_TRUE if this array is enabled. Otherwise, it is set
>
More information about the mesa-dev
mailing list