This patch is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
On 06/23/2016 11:15 AM, Matt Turner wrote: > From: Colin McDonald <cjmmail10...@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 <matts...@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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
