From a4a666152b50ee4b489f0fba56c2cef082a9157d Mon Sep 17 00:00:00 2001
From: Jelle Raaijmakers <jelle@gmta.nl>
Date: Tue, 11 Jan 2022 00:50:17 +0100
Subject: [PATCH] LibGfx+LibGL: Do not crash if matrix inverse does not exist

Allow `Matrix::inverse()` to return an error and deal with those in
LibGL. Also use this opportunity to more efficiently calculate the
transpose of the model view matrix for the normal transformation.
---
 Userland/Libraries/LibGL/SoftwareGLContext.cpp | 17 ++++++++++-------
 Userland/Libraries/LibGfx/Matrix.h             | 12 ++++++------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Userland/Libraries/LibGL/SoftwareGLContext.cpp b/Userland/Libraries/LibGL/SoftwareGLContext.cpp
index 5fa8fe88e3..27d2f32215 100644
--- a/Userland/Libraries/LibGL/SoftwareGLContext.cpp
+++ b/Userland/Libraries/LibGL/SoftwareGLContext.cpp
@@ -313,11 +313,12 @@ void SoftwareGLContext::gl_end()
     // Set up normals transform by taking the upper left 3x3 elements from the model view matrix
     // See section 2.11.3 of the OpenGL 1.5 spec
     auto const& mv_elements = m_model_view_matrix.elements();
-    auto normal_transform = FloatMatrix3x3(
-        mv_elements[0][0], mv_elements[0][1], mv_elements[0][2],
-        mv_elements[1][0], mv_elements[1][1], mv_elements[1][2],
-        mv_elements[2][0], mv_elements[2][1], mv_elements[2][2]);
-    normal_transform = normal_transform.inverse();
+    auto const model_view_transposed = FloatMatrix3x3(
+        mv_elements[0][0], mv_elements[1][0], mv_elements[2][0],
+        mv_elements[0][1], mv_elements[1][1], mv_elements[2][1],
+        mv_elements[0][2], mv_elements[1][2], mv_elements[2][2]);
+    auto normal_transform_or_error = model_view_transposed.inverse();
+    auto const& normal_transform = normal_transform_or_error.is_error() ? model_view_transposed : normal_transform_or_error.release_value();
 
     m_rasterizer.draw_primitives(primitive_type, m_model_view_matrix, normal_transform, m_projection_matrix, m_texture_matrix, m_vertex_list, enabled_texture_units);
 
@@ -2809,7 +2810,9 @@ void SoftwareGLContext::gl_tex_gen_floatv(GLenum coord, GLenum pname, GLfloat co
         texture_coordinate_generation(capability).object_plane_coefficients = { params[0], params[1], params[2], params[3] };
         break;
     case GL_EYE_PLANE: {
-        auto inverted_model_view_matrix = m_model_view_matrix.inverse();
+        auto inverse_matrix_or_error = m_model_view_matrix.inverse();
+        auto const& inverse_model_view_matrix = inverse_matrix_or_error.is_error() ? m_model_view_matrix : inverse_matrix_or_error.release_value();
+
         auto input_coefficients = FloatVector4 { params[0], params[1], params[2], params[3] };
 
         // Note: we are allowed to store transformed coefficients here, according to the documentation on
@@ -2818,7 +2821,7 @@ void SoftwareGLContext::gl_tex_gen_floatv(GLenum coord, GLenum pname, GLfloat co
         // "The returned values are those maintained in eye coordinates. They are not equal to the values
         //  specified using glTexGen, unless the modelview matrix was identity when glTexGen was called."
 
-        texture_coordinate_generation(capability).eye_plane_coefficients = inverted_model_view_matrix * input_coefficients;
+        texture_coordinate_generation(capability).eye_plane_coefficients = inverse_model_view_matrix * input_coefficients;
         break;
     }
     default:
diff --git a/Userland/Libraries/LibGfx/Matrix.h b/Userland/Libraries/LibGfx/Matrix.h
index 427c43b8b9..33f9536ce2 100644
--- a/Userland/Libraries/LibGfx/Matrix.h
+++ b/Userland/Libraries/LibGfx/Matrix.h
@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/Error.h>
 #include <AK/Types.h>
 #include <initializer_list>
 
@@ -85,9 +86,8 @@ public:
     {
         Matrix division;
         for (size_t i = 0; i < N; ++i) {
-            for (size_t j = 0; j < N; ++j) {
+            for (size_t j = 0; j < N; ++j)
                 division.m_elements[i][j] = m_elements[i][j] / divisor;
-            }
         }
         return division;
     }
@@ -159,10 +159,11 @@ public:
         return result;
     }
 
-    [[nodiscard]] constexpr Matrix inverse() const
+    [[nodiscard]] constexpr ErrorOr<Matrix> inverse() const
     {
         auto det = determinant();
-        VERIFY(det != 0);
+        if (det == 0)
+            return Error::from_string_literal("inverse of matrix does not exist"sv);
         return adjugate() / det;
     }
 
@@ -170,9 +171,8 @@ public:
     {
         Matrix result;
         for (size_t i = 0; i < N; ++i) {
-            for (size_t j = 0; j < N; ++j) {
+            for (size_t j = 0; j < N; ++j)
                 result.m_elements[i][j] = m_elements[j][i];
-            }
         }
         return result;
     }