yuq / mesa-lima

Deprecated, new place: https://gitlab.freedesktop.org/lima
https://github.com/yuq/mesa-lima/wiki
165 stars 18 forks source link

lima: fix indexed draw with glDrawElements #31

Closed enunes closed 6 years ago

enunes commented 6 years ago

This implements the indexed draw handling in lima_pack_plbu_cmd() during the vbo draw. Without this patch, lima attempts to use info->index.resource even when info->has_user_indices is set, which causes a segfault in mesa. Instead, the driver needs to first upload the raw indices buffer contained in info->index.user_buffer before attempting to access info->index.resource.

Signed-off-by: Erico Nunes nunes.erico@gmail.com

enunes commented 6 years ago

I checked this with the following test patch in kmscube:

diff --git a/cube-smooth.c b/cube-smooth.c
index f29face..f77fb7e 100644
--- a/cube-smooth.c
+++ b/cube-smooth.c
@@ -210,12 +210,20 @@ static void draw_cube_smooth(unsigned i)
        glUniformMatrix4fv(gl.modelviewprojectionmatrix, 1, GL_FALSE, &modelviewprojection.m[0][0]);
        glUniformMatrix3fv(gl.normalmatrix, 1, GL_FALSE, normal);

-       glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
-       glDrawArrays(GL_TRIANGLE_STRIP, 4, 4);
-       glDrawArrays(GL_TRIANGLE_STRIP, 8, 4);
-       glDrawArrays(GL_TRIANGLE_STRIP, 12, 4);
-       glDrawArrays(GL_TRIANGLE_STRIP, 16, 4);
-       glDrawArrays(GL_TRIANGLE_STRIP, 20, 4);
+       GLushort indices[] = {  0,  1,  2,
+                               2,  1,  3,
+                               4,  5,  6,
+                               6,  5,  7,
+                               8,  9, 10,
+                              10,  9, 11,
+                              12, 13, 14,
+                              14, 13, 15,
+                              16, 17, 18,
+                              18, 17, 19,
+                              20, 21, 22,
+                              22, 21, 23, };
+
+       glDrawElements(GL_TRIANGLES, sizeof(indices)/sizeof(indices[0]), GL_UNSIGNED_SHORT, indices);
 }

 const struct egl * init_cube_smooth(const struct gbm *gbm)
enunes commented 6 years ago

Under some circumstances, with multiple glDrawElements calls, I still get nothing being rendered sometimes. It seems to depend on how the vertices are accessed, for example accessing the first elements of the vertex array in the first glDrawElements call or a further call. Even when this happens, the indices buffer setup, its virtual address, the contents of the mapped buffer and the plbu setup appear to be correct though, so I wonder if that might be somewhat unrelated. @yuq let me know if you can think of something that might cause that.

enunes commented 6 years ago

I did the changes you suggested and it looks better now. Not sure why I was getting a warning about the const info or why freedreno uses new_info then, but still it works.

However, even the lima_submit_add_bo still did not solve the issue with no rendering sometimes in multiple glDrawElements calls. The indices still seem to be correctly present in res->bo->va + index_offset (I checked that by dumping the res->bo->map + index_offset), so not sure what other problem we may have.

yuq commented 6 years ago

These two commits are also needed for not free buffer before task is done: https://github.com/yuq/mesa-lima/commit/67647dae69b9759e61640f76a2db63a28e9b1987 https://github.com/yuq/linux-lima/commit/b7cf8f44dafffd77d62a602bb0242cd906674d19

enunes commented 6 years ago

thanks for the review @anarsoul , this will be fixed in the next spin.

This patch is currently stalled since we had questions about min_index and max_index which to be solved properly may require a change in mesa, so we are discussing it in the upstream mailing list. After we get the feedback from the list I will proceed with this.

enunes commented 6 years ago

I created a new PR for the 18.0 branch, so I'm closing this one.