From 220ac5eb026c8e1fd01af0544c0e367c886eab80 Mon Sep 17 00:00:00 2001 From: Stephan Unverwerth Date: Mon, 16 Aug 2021 19:22:08 +0200 Subject: [PATCH] LibGL: Fix clipping and interpolate vertex attributes The previous clipping implementation was problematic especially when clipping against the near plane. Triangles are now correctly clipped using homogenous coordinates against all frustum planes. Texture coordinates and vertex colors are now correctly interpolated. The earier implementation was just a placeholder. --- Userland/Libraries/LibGL/Clipper.cpp | 105 +++++------- Userland/Libraries/LibGL/Clipper.h | 23 ++- Userland/Libraries/LibGL/GLStruct.h | 8 +- .../Libraries/LibGL/SoftwareGLContext.cpp | 152 +++++------------- Userland/Libraries/LibGL/SoftwareGLContext.h | 1 + .../Libraries/LibGL/SoftwareRasterizer.cpp | 27 ++-- 6 files changed, 111 insertions(+), 205 deletions(-) diff --git a/Userland/Libraries/LibGL/Clipper.cpp b/Userland/Libraries/LibGL/Clipper.cpp index 5fc7cb298c1..17114491ff0 100644 --- a/Userland/Libraries/LibGL/Clipper.cpp +++ b/Userland/Libraries/LibGL/Clipper.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2021, Jesse Buhagiar + * Copyright (c) 2021, Stephan Unverwerth * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,7 +9,9 @@ #include #include -bool GL::Clipper::point_within_clip_plane(const FloatVector4& vertex, ClipPlane plane) +namespace GL { + +bool Clipper::point_within_clip_plane(const FloatVector4& vertex, ClipPlane plane) { switch (plane) { case ClipPlane::LEFT: @@ -28,83 +31,53 @@ bool GL::Clipper::point_within_clip_plane(const FloatVector4& vertex, ClipPlane return false; } -// TODO: This needs to interpolate color/UV data as well! -FloatVector4 GL::Clipper::clip_intersection_point(const FloatVector4& vec, const FloatVector4& prev_vec, ClipPlane plane_index) +GLVertex Clipper::clip_intersection_point(const GLVertex& p1, const GLVertex& p2, ClipPlane plane_index) { + // See https://www.microsoft.com/en-us/research/wp-content/uploads/1978/01/p245-blinn.pdf + // "Clipping Using Homogenous Coordinates" Blinn/Newell, 1978 - // - // This is an implementation of the Cyrus-Beck algorithm to clip lines against a plane - // using the triangle's line segment in parametric form. - // In this case, we find that n . [P(t) - plane] == 0 if the point lies on - // the boundary, which in this case is the clip plane. We then substitute - // P(t)= P1 + (P2-P1)*t (where P2 is a point inside the clipping boundary, and P1 is, - // in this case, the point that lies outside the boundary due to our implementation of Sutherland-Hogdman) - // into P(t) to arrive at the equation: - // - // n · [P1 + ((P2 - P1) * t) - plane] = 0. - // Substitude seg_length = P2 - P1 (length of line segment) and dist = P1 - plane (distance from P1 to plane) - // - // By rearranging, we now end up with - // - // n·[P1 + (seg_length * t) - plane] = 0 - // n·(dist) + n·(seg_length * t) = 0 - // n·(seg_length*t) = -n·(dist) - // Therefore - // t = (-n·(dist))/(n·seg_length) - // - // NOTE: n is the normal vector to the plane we are clipping against. - // - // Proof of algorithm found here - // http://studymaterial.unipune.ac.in:8080/jspui/bitstream/123456789/4843/1/Cyrus_Beck_Algo.pdf - // And here (slightly more intuitive with a better diagram) - // https://www.csee.umbc.edu/~rheingan/435/pages/res/gen-5.Clipping-single-page-0.html + float w1 = p1.position.w(); + float w2 = p2.position.w(); + float x1 = clip_plane_normals[plane_index].dot(p1.position); + float x2 = clip_plane_normals[plane_index].dot(p2.position); + float a = (w1 + x1) / ((w1 + x1) - (w2 + x2)); - FloatVector4 seg_length = vec - prev_vec; // P2 - P1 - FloatVector4 dist = prev_vec - clip_planes[plane_index]; // Distance from "out of bounds" vector to plane - - float plane_normal_line_segment_dot_product = clip_plane_normals[plane_index].dot(seg_length); - float neg_plane_normal_dist_dot_procut = -clip_plane_normals[plane_index].dot(dist); - float t = (plane_normal_line_segment_dot_product / neg_plane_normal_dist_dot_procut); - - // P(t) = P1 + (t * (P2 - P1)) - FloatVector4 lerped_vec = prev_vec + (seg_length * t); - - return lerped_vec; + GLVertex out; + out.position = p1.position * (1 - a) + p2.position * a; + out.color = p1.color * (1 - a) + p2.color * a; + out.tex_coord = p1.tex_coord * (1 - a) + p2.tex_coord * a; + return out; } -// FIXME: Getting too close to zNear causes VERY serious artifacting. Should we cull the whole triangle?? -void GL::Clipper::clip_triangle_against_frustum(Vector& input_verts) +void Clipper::clip_triangle_against_frustum(Vector& input_verts) { - Vector clipped_polygon; - Vector input = input_verts; - Vector* current = &clipped_polygon; - Vector* output_list = &input; - ScopeGuard guard { [&] { input_verts = *output_list; } }; + list_a = input_verts; + list_b.clear_with_capacity(); - for (u8 plane = 0; plane < NUMBER_OF_CLIPPING_PLANES; plane++) { - swap(current, output_list); - clipped_polygon.clear(); - - if (current->size() == 0) { - return; - } + auto read_from = &list_a; + auto write_to = &list_b; + for (size_t plane = 0; plane < NUMBER_OF_CLIPPING_PLANES; plane++) { + write_to->clear_with_capacity(); // Save me, C++23 - for (size_t i = 0; i < current->size(); i++) { - const auto& curr_vec = current->at(i); - const auto& prev_vec = current->at((i - 1) % current->size()); + for (size_t i = 0; i < read_from->size(); i++) { + const auto& curr_vec = read_from->at((i + 1) % read_from->size()); + const auto& prev_vec = read_from->at(i); - if (point_within_clip_plane(curr_vec, static_cast(plane))) { - if (!point_within_clip_plane(prev_vec, static_cast(plane))) { - FloatVector4 intersect = clip_intersection_point(prev_vec, curr_vec, static_cast(plane)); - clipped_polygon.append(intersect); + if (point_within_clip_plane(curr_vec.position, static_cast(plane))) { + if (!point_within_clip_plane(prev_vec.position, static_cast(plane))) { + auto intersect = clip_intersection_point(prev_vec, curr_vec, static_cast(plane)); + write_to->append(intersect); } - - clipped_polygon.append(curr_vec); - } else if (point_within_clip_plane(prev_vec, static_cast(plane))) { - FloatVector4 intersect = clip_intersection_point(prev_vec, curr_vec, static_cast(plane)); - clipped_polygon.append(intersect); + write_to->append(curr_vec); + } else if (point_within_clip_plane(prev_vec.position, static_cast(plane))) { + auto intersect = clip_intersection_point(prev_vec, curr_vec, static_cast(plane)); + write_to->append(intersect); } } + swap(write_to, read_from); } + + input_verts = *read_from; +} } diff --git a/Userland/Libraries/LibGL/Clipper.h b/Userland/Libraries/LibGL/Clipper.h index 5c2e5bb2768..36452a6e214 100644 --- a/Userland/Libraries/LibGL/Clipper.h +++ b/Userland/Libraries/LibGL/Clipper.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include namespace GL { @@ -34,26 +35,24 @@ class Clipper final { }; static constexpr FloatVector4 clip_plane_normals[] = { - { 1, 0, 0, 1 }, // Left Plane - { -1, 0, 0, 1 }, // Right Plane - { 0, -1, 0, 1 }, // Top Plane - { 0, 1, 0, 1 }, // Bottom plane - { 0, 0, -1, 1 }, // Near Plane - { 0, 0, 1, 1 } // Far Plane + { 1, 0, 0, 0 }, // Left Plane + { -1, 0, 0, 0 }, // Right Plane + { 0, -1, 0, 0 }, // Top Plane + { 0, 1, 0, 0 }, // Bottom plane + { 0, 0, 1, 0 }, // Near Plane + { 0, 0, -1, 0 } // Far Plane }; public: Clipper() { } - void clip_triangle_against_frustum(Vector& input_vecs); - const Vector& clipped_verts() const; + void clip_triangle_against_frustum(Vector& input_vecs); private: bool point_within_clip_plane(const FloatVector4& vertex, ClipPlane plane); - FloatVector4 clip_intersection_point(const FloatVector4& vec, const FloatVector4& prev_vec, ClipPlane plane_index); - -private: - Vector m_clipped_verts; + GLVertex clip_intersection_point(const GLVertex& vec, const GLVertex& prev_vec, ClipPlane plane_index); + Vector list_a; + Vector list_b; }; } diff --git a/Userland/Libraries/LibGL/GLStruct.h b/Userland/Libraries/LibGL/GLStruct.h index ded910aa08c..9b7785462fd 100644 --- a/Userland/Libraries/LibGL/GLStruct.h +++ b/Userland/Libraries/LibGL/GLStruct.h @@ -7,6 +7,8 @@ #pragma once #include "GL/gl.h" +#include +#include namespace GL { @@ -15,9 +17,9 @@ struct GLColor { }; struct GLVertex { - GLfloat x, y, z, w; - GLfloat r, g, b, a; - GLfloat u, v; + FloatVector4 position; + FloatVector4 color; + FloatVector2 tex_coord; }; struct GLTriangle { diff --git a/Userland/Libraries/LibGL/SoftwareGLContext.cpp b/Userland/Libraries/LibGL/SoftwareGLContext.cpp index c24e450030d..70355668d28 100644 --- a/Userland/Libraries/LibGL/SoftwareGLContext.cpp +++ b/Userland/Libraries/LibGL/SoftwareGLContext.cpp @@ -123,6 +123,9 @@ void SoftwareGLContext::gl_end() // Make sure we had a `glBegin` before this call... RETURN_WITH_ERROR_IF(!m_in_draw_state, GL_INVALID_OPERATION); + triangle_list.clear_with_capacity(); + processed_triangles.clear_with_capacity(); + // Let's construct some triangles if (m_current_draw_mode == GL_TRIANGLES) { GLTriangle triangle; @@ -169,29 +172,22 @@ void SoftwareGLContext::gl_end() triangle_list.append(triangle); } } else { + vertex_list.clear_with_capacity(); RETURN_WITH_ERROR_IF(true, GL_INVALID_ENUM); } + vertex_list.clear_with_capacity(); + + auto mvp = m_projection_matrix * m_model_view_matrix; + // Now let's transform each triangle and send that to the GPU for (size_t i = 0; i < triangle_list.size(); i++) { GLTriangle& triangle = triangle_list.at(i); - GLVertex& vertexa = triangle.vertices[0]; - GLVertex& vertexb = triangle.vertices[1]; - GLVertex& vertexc = triangle.vertices[2]; - - FloatVector4 veca({ vertexa.x, vertexa.y, vertexa.z, 1.0f }); - FloatVector4 vecb({ vertexb.x, vertexb.y, vertexb.z, 1.0f }); - FloatVector4 vecc({ vertexc.x, vertexc.y, vertexc.z, 1.0f }); // First multiply the vertex by the MODELVIEW matrix and then the PROJECTION matrix - veca = m_model_view_matrix * veca; - veca = m_projection_matrix * veca; - - vecb = m_model_view_matrix * vecb; - vecb = m_projection_matrix * vecb; - - vecc = m_model_view_matrix * vecc; - vecc = m_projection_matrix * vecc; + triangle.vertices[0].position = mvp * triangle.vertices[0].position; + triangle.vertices[1].position = mvp * triangle.vertices[1].position; + triangle.vertices[2].position = mvp * triangle.vertices[2].position; // At this point, we're in clip space // Here's where we do the clipping. This is a really crude implementation of the @@ -203,87 +199,35 @@ void SoftwareGLContext::gl_end() // Okay, let's do some face culling first - Vector vecs; - Vector verts; + m_clipped_vertices.clear_with_capacity(); + m_clipped_vertices.append(triangle.vertices[0]); + m_clipped_vertices.append(triangle.vertices[1]); + m_clipped_vertices.append(triangle.vertices[2]); + m_clipper.clip_triangle_against_frustum(m_clipped_vertices); - vecs.append(veca); - vecs.append(vecb); - vecs.append(vecc); - m_clipper.clip_triangle_against_frustum(vecs); + if (m_clipped_vertices.size() < 3) + continue; - // TODO: Copy color and UV information too! - for (size_t vec_idx = 0; vec_idx < vecs.size(); vec_idx++) { - FloatVector4& vec = vecs.at(vec_idx); - GLVertex vertex; + for (auto& vec : m_clipped_vertices) { + // perspective divide + float w = vec.position.w(); + vec.position.set_x(vec.position.x() / w); + vec.position.set_y(vec.position.y() / w); + vec.position.set_z(vec.position.z() / w); + vec.position.set_w(1 / w); - // Perform the perspective divide - if (vec.w() != 0.0f) { - vec.set_x(vec.x() / vec.w()); - vec.set_y(vec.y() / vec.w()); - vec.set_z(vec.z() / vec.w()); - vec.set_w(1 / vec.w()); - } - - vertex.x = vec.x(); - vertex.y = vec.y(); - vertex.z = vec.z(); - vertex.w = vec.w(); - - // FIXME: This is to suppress any -Wunused errors - vertex.u = 0.0f; - vertex.v = 0.0f; - - if (vec_idx == 0) { - vertex.r = vertexa.r; - vertex.g = vertexa.g; - vertex.b = vertexa.b; - vertex.a = vertexa.a; - vertex.u = vertexa.u; - vertex.v = vertexa.v; - } else if (vec_idx == 1) { - vertex.r = vertexb.r; - vertex.g = vertexb.g; - vertex.b = vertexb.b; - vertex.a = vertexb.a; - vertex.u = vertexb.u; - vertex.v = vertexb.v; - } else { - vertex.r = vertexc.r; - vertex.g = vertexc.g; - vertex.b = vertexc.b; - vertex.a = vertexc.a; - vertex.u = vertexc.u; - vertex.v = vertexc.v; - } - - vertex.x = (vec.x() + 1.0f) * (scr_width / 2.0f) + 0.0f; // TODO: 0.0f should be something!? - vertex.y = scr_height - ((vec.y() + 1.0f) * (scr_height / 2.0f) + 0.0f); - vertex.z = vec.z(); - verts.append(vertex); + // to screen space + vec.position.set_x(scr_width / 2 + vec.position.x() * scr_width / 2); + vec.position.set_y(scr_height / 2 - vec.position.y() * scr_height / 2); } - if (verts.size() == 0) { - continue; - } else if (verts.size() == 3) { - GLTriangle tri; + GLTriangle tri; + tri.vertices[0] = m_clipped_vertices[0]; + for (size_t i = 1; i < m_clipped_vertices.size() - 1; i++) { - tri.vertices[0] = verts.at(0); - tri.vertices[1] = verts.at(1); - tri.vertices[2] = verts.at(2); + tri.vertices[1] = m_clipped_vertices[i]; + tri.vertices[2] = m_clipped_vertices[i + 1]; processed_triangles.append(tri); - } else if (verts.size() == 4) { - GLTriangle tri1; - GLTriangle tri2; - - tri1.vertices[0] = verts.at(0); - tri1.vertices[1] = verts.at(1); - tri1.vertices[2] = verts.at(2); - processed_triangles.append(tri1); - - tri2.vertices[0] = verts.at(0); - tri2.vertices[1] = verts.at(2); - tri2.vertices[2] = verts.at(3); - processed_triangles.append(tri2); } } @@ -292,10 +236,10 @@ void SoftwareGLContext::gl_end() // Let's calculate the (signed) area of the triangle // https://cp-algorithms.com/geometry/oriented-triangle-area.html - float dxAB = triangle.vertices[0].x - triangle.vertices[1].x; // A.x - B.x - float dxBC = triangle.vertices[1].x - triangle.vertices[2].x; // B.X - C.x - float dyAB = triangle.vertices[0].y - triangle.vertices[1].y; - float dyBC = triangle.vertices[1].y - triangle.vertices[2].y; + float dxAB = triangle.vertices[0].position.x() - triangle.vertices[1].position.x(); // A.x - B.x + float dxBC = triangle.vertices[1].position.x() - triangle.vertices[2].position.x(); // B.X - C.x + float dyAB = triangle.vertices[0].position.y() - triangle.vertices[1].position.y(); + float dyBC = triangle.vertices[1].position.y() - triangle.vertices[2].position.y(); float area = (dxAB * dyBC) - (dxBC * dyAB); if (area == 0.0f) @@ -314,10 +258,6 @@ void SoftwareGLContext::gl_end() m_rasterizer.submit_triangle(triangle, m_texture_units); } - triangle_list.clear(); - processed_triangles.clear(); - vertex_list.clear(); - m_in_draw_state = false; } @@ -546,19 +486,9 @@ void SoftwareGLContext::gl_vertex(GLdouble x, GLdouble y, GLdouble z, GLdouble w GLVertex vertex; - vertex.x = x; - vertex.y = y; - vertex.z = z; - vertex.w = w; - vertex.r = m_current_vertex_color.x(); - vertex.g = m_current_vertex_color.y(); - vertex.b = m_current_vertex_color.z(); - vertex.a = m_current_vertex_color.w(); - - // FIXME: This is to suppress any -Wunused errors - vertex.w = 0.0f; - vertex.u = m_current_vertex_tex_coord.x(); - vertex.v = m_current_vertex_tex_coord.y(); + vertex.position = { static_cast(x), static_cast(y), static_cast(z), static_cast(w) }; + vertex.color = m_current_vertex_color; + vertex.tex_coord = { m_current_vertex_tex_coord.x(), m_current_vertex_tex_coord.y() }; vertex_list.append(vertex); } @@ -847,7 +777,7 @@ void SoftwareGLContext::gl_delete_lists(GLuint list, GLsizei range) return; for (auto& entry : m_listings.span().slice(list - 1, range)) - entry.entries.clear(); + entry.entries.clear_with_capacity(); } void SoftwareGLContext::gl_end_list() diff --git a/Userland/Libraries/LibGL/SoftwareGLContext.h b/Userland/Libraries/LibGL/SoftwareGLContext.h index f5055cb530c..18e64497200 100644 --- a/Userland/Libraries/LibGL/SoftwareGLContext.h +++ b/Userland/Libraries/LibGL/SoftwareGLContext.h @@ -126,6 +126,7 @@ private: Vector vertex_list; Vector triangle_list; Vector processed_triangles; + Vector m_clipped_vertices; GLenum m_error = GL_NO_ERROR; bool m_in_draw_state = false; diff --git a/Userland/Libraries/LibGL/SoftwareRasterizer.cpp b/Userland/Libraries/LibGL/SoftwareRasterizer.cpp index b134f4a30ef..ea4e019418a 100644 --- a/Userland/Libraries/LibGL/SoftwareRasterizer.cpp +++ b/Userland/Libraries/LibGL/SoftwareRasterizer.cpp @@ -107,9 +107,9 @@ static void rasterize_triangle(const RasterizerOptions& options, Gfx::Bitmap& re VERIFY((render_target.height() % RASTERIZER_BLOCK_SIZE) == 0); // Calculate area of the triangle for later tests - IntVector2 v0 { (int)triangle.vertices[0].x, (int)triangle.vertices[0].y }; - IntVector2 v1 { (int)triangle.vertices[1].x, (int)triangle.vertices[1].y }; - IntVector2 v2 { (int)triangle.vertices[2].x, (int)triangle.vertices[2].y }; + IntVector2 v0 { (int)triangle.vertices[0].position.x(), (int)triangle.vertices[0].position.y() }; + IntVector2 v1 { (int)triangle.vertices[1].position.x(), (int)triangle.vertices[1].position.y() }; + IntVector2 v2 { (int)triangle.vertices[2].position.x(), (int)triangle.vertices[2].position.y() }; int area = edge_function(v0, v1, v2); if (area == 0) @@ -259,7 +259,8 @@ static void rasterize_triangle(const RasterizerOptions& options, Gfx::Bitmap& re continue; auto barycentric = FloatVector3(coords.x(), coords.y(), coords.z()) * one_over_area; - float z = interpolate(triangle.vertices[0].z, triangle.vertices[1].z, triangle.vertices[2].z, barycentric); + float z = interpolate(triangle.vertices[0].position.z(), triangle.vertices[1].position.z(), triangle.vertices[2].position.z(), barycentric); + z = options.depth_min + (options.depth_max - options.depth_min) * (z + 1) / 2; bool pass = false; @@ -322,26 +323,26 @@ static void rasterize_triangle(const RasterizerOptions& options, Gfx::Bitmap& re // Perspective correct barycentric coordinates auto barycentric = FloatVector3(coords.x(), coords.y(), coords.z()) * one_over_area; - float interpolated_reciprocal_w = interpolate(triangle.vertices[0].w, triangle.vertices[1].w, triangle.vertices[2].w, barycentric); + float interpolated_reciprocal_w = interpolate(triangle.vertices[0].position.w(), triangle.vertices[1].position.w(), triangle.vertices[2].position.w(), barycentric); float interpolated_w = 1 / interpolated_reciprocal_w; - barycentric = barycentric * FloatVector3(triangle.vertices[0].w, triangle.vertices[1].w, triangle.vertices[2].w) * interpolated_w; + barycentric = barycentric * FloatVector3(triangle.vertices[0].position.w(), triangle.vertices[1].position.w(), triangle.vertices[2].position.w()) * interpolated_w; // FIXME: make this more generic. We want to interpolate more than just color and uv FloatVector4 vertex_color; if (options.shade_smooth) { vertex_color = interpolate( - FloatVector4(triangle.vertices[0].r, triangle.vertices[0].g, triangle.vertices[0].b, triangle.vertices[0].a), - FloatVector4(triangle.vertices[1].r, triangle.vertices[1].g, triangle.vertices[1].b, triangle.vertices[1].a), - FloatVector4(triangle.vertices[2].r, triangle.vertices[2].g, triangle.vertices[2].b, triangle.vertices[2].a), + triangle.vertices[0].color, + triangle.vertices[1].color, + triangle.vertices[2].color, barycentric); } else { - vertex_color = { triangle.vertices[0].r, triangle.vertices[0].g, triangle.vertices[0].b, triangle.vertices[0].a }; + vertex_color = triangle.vertices[0].color; } auto uv = interpolate( - FloatVector2(triangle.vertices[0].u, triangle.vertices[0].v), - FloatVector2(triangle.vertices[1].u, triangle.vertices[1].v), - FloatVector2(triangle.vertices[2].u, triangle.vertices[2].v), + triangle.vertices[0].tex_coord, + triangle.vertices[1].tex_coord, + triangle.vertices[2].tex_coord, barycentric); *pixel = pixel_shader(uv, vertex_color);