From 0cf3cb62797eeb6f45bb3a9d32f54cdb3df89b29 Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Sun, 16 Oct 2022 19:14:53 +0200 Subject: [PATCH] LibGL: Immediately dereference vertex attribute data in display lists According to the spec, pointers to client data need to be dereferenced immediately when adding calls such as `glDrawElements` or `glArrayElement` to a display list. We were trying to support display lists for these calls but since they only invoke _other_ calls that also support display lists, we can simply defer the display list functionality to them. This fixes the rendering of the ClassiCube port by cflip. --- Tests/LibGL/TestRender.cpp | 26 ++++++++++++++++++ ...009_test_draw_elements_in_display_list.qoi | Bin 0 -> 184 bytes Userland/Libraries/LibGL/GLContext.h | 3 -- Userland/Libraries/LibGL/Vertex.cpp | 9 ++++-- 4 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 Tests/LibGL/reference-images/0009_test_draw_elements_in_display_list.qoi diff --git a/Tests/LibGL/TestRender.cpp b/Tests/LibGL/TestRender.cpp index 240f8f23e1..6c26212210 100644 --- a/Tests/LibGL/TestRender.cpp +++ b/Tests/LibGL/TestRender.cpp @@ -252,3 +252,29 @@ TEST_CASE(0008_test_pop_matrix_regression) context->present(); expect_bitmap_equals_reference(context->frontbuffer(), "0008_test_pop_matrix_regression"sv); } + +TEST_CASE(0009_test_draw_elements_in_display_list) +{ + auto context = create_testing_context(64, 64); + + glColor3f(0.f, 0.f, 1.f); + glEnableClientState(GL_VERTEX_ARRAY); + + auto const list_index = glGenLists(1); + glNewList(list_index, GL_COMPILE); + float vertices[] = { 0.f, .5f, -.5f, -.5f, .5f, -.5f }; + glVertexPointer(2, GL_FLOAT, 0, &vertices); + u8 indices[] = { 0, 1, 2 }; + glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_BYTE, &indices); + glEndList(); + + // Modifying an index here should not have an effect + indices[0] = 2; + + glCallList(list_index); + + EXPECT_EQ(glGetError(), 0u); + + context->present(); + expect_bitmap_equals_reference(context->frontbuffer(), "0009_test_draw_elements_in_display_list"sv); +} diff --git a/Tests/LibGL/reference-images/0009_test_draw_elements_in_display_list.qoi b/Tests/LibGL/reference-images/0009_test_draw_elements_in_display_list.qoi new file mode 100644 index 0000000000000000000000000000000000000000..1b4763e8847391c3fabb674190ef532b9f4ff975 GIT binary patch literal 184 zcmXTS&rD-rU~m9o7KXnV;Ltw?hW`gl|L7es{jGP%^q1Zt)1P`rOn>MdG5xN0%=DYy zG1ISlCrrQSoiP2ZcgpmW-YL_MdS^^O=$$cruXoP$o!&Xqw|W;$-{@U1eXV!N^p)Nv z)0cWzOke0-F@3Ig&GebxHPfehH%y=C-7tNucgysV-YwIIdUs48=-n~BuXoQB-97IZ HAb=47Yt?8X literal 0 HcmV?d00001 diff --git a/Userland/Libraries/LibGL/GLContext.h b/Userland/Libraries/LibGL/GLContext.h index 6f780991dd..e068094ba2 100644 --- a/Userland/Libraries/LibGL/GLContext.h +++ b/Userland/Libraries/LibGL/GLContext.h @@ -441,8 +441,6 @@ private: decltype(&GLContext::gl_tex_parameter), decltype(&GLContext::gl_tex_parameterfv), decltype(&GLContext::gl_depth_mask), - decltype(&GLContext::gl_draw_arrays), - decltype(&GLContext::gl_draw_elements), decltype(&GLContext::gl_draw_pixels), decltype(&GLContext::gl_depth_range), decltype(&GLContext::gl_polygon_offset), @@ -473,7 +471,6 @@ private: decltype(&GLContext::gl_color_material), decltype(&GLContext::gl_get_light), decltype(&GLContext::gl_clip_plane), - decltype(&GLContext::gl_array_element), decltype(&GLContext::gl_copy_tex_sub_image_2d), decltype(&GLContext::gl_point_size)>; diff --git a/Userland/Libraries/LibGL/Vertex.cpp b/Userland/Libraries/LibGL/Vertex.cpp index 5e5308fd92..e35c0ab5ff 100644 --- a/Userland/Libraries/LibGL/Vertex.cpp +++ b/Userland/Libraries/LibGL/Vertex.cpp @@ -14,7 +14,8 @@ namespace GL { void GLContext::gl_array_element(GLint i) { - APPEND_TO_CALL_LIST_AND_RETURN_IF_NEEDED(gl_array_element, i); + // NOTE: This always dereferences data; display list support is deferred to the + // individual vertex attribute calls such as `gl_color`, `gl_normal` etc. RETURN_WITH_ERROR_IF(i < 0, GL_INVALID_VALUE); // This is effectively the same as `gl_draw_elements`, except we only output a single @@ -79,7 +80,8 @@ void GLContext::gl_color_pointer(GLint size, GLenum type, GLsizei stride, void c void GLContext::gl_draw_arrays(GLenum mode, GLint first, GLsizei count) { - APPEND_TO_CALL_LIST_AND_RETURN_IF_NEEDED(gl_draw_arrays, mode, first, count); + // NOTE: This always dereferences data; display list support is deferred to the + // individual vertex attribute calls such as `gl_color`, `gl_normal` etc. RETURN_WITH_ERROR_IF(m_in_draw_state, GL_INVALID_OPERATION); // FIXME: Some modes are still missing (GL_POINTS, GL_LINE_STRIP, GL_LINE_LOOP, GL_LINES) @@ -129,7 +131,8 @@ void GLContext::gl_draw_arrays(GLenum mode, GLint first, GLsizei count) void GLContext::gl_draw_elements(GLenum mode, GLsizei count, GLenum type, void const* indices) { - APPEND_TO_CALL_LIST_AND_RETURN_IF_NEEDED(gl_draw_elements, mode, count, type, indices); + // NOTE: This always dereferences data; display list support is deferred to the + // individual vertex attribute calls such as `gl_color`, `gl_normal` etc. RETURN_WITH_ERROR_IF(m_in_draw_state, GL_INVALID_OPERATION); // FIXME: Some modes are still missing (GL_POINTS, GL_LINE_STRIP, GL_LINE_LOOP, GL_LINES)