From edcb6176ce51fa73890c9886b7d715842811f0d3 Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Thu, 5 Oct 2023 22:01:35 +0200 Subject: [PATCH] LibGL+Lib*GPU: Set model view and projection matrices separately LibSoftGPU used to calculate the normal transformation based on the model view transformation for every primitive, because that's when we sent over the matrix. By making LibGL a bit smarter and only update the matrices when they could have changed, we only need to calculate the normal transformation once on every matrix update. When viewing `Tuba.obj` in 3DFileViewer, this brings the percentage of time spent in `FloatMatrix4x4::inverse()` down from 15% to 0%. :^) --- Userland/Libraries/LibGL/GLContext.cpp | 8 +++--- Userland/Libraries/LibGL/GLContext.h | 5 +++- Userland/Libraries/LibGL/Matrix.cpp | 13 ++++++++++ Userland/Libraries/LibGPU/Device.h | 6 +++-- Userland/Libraries/LibSoftGPU/Device.cpp | 32 ++++++++++++++++-------- Userland/Libraries/LibSoftGPU/Device.h | 10 ++++++-- Userland/Libraries/LibVirtGPU/Device.cpp | 16 +++++++++--- Userland/Libraries/LibVirtGPU/Device.h | 9 +++++-- 8 files changed, 75 insertions(+), 24 deletions(-) diff --git a/Userland/Libraries/LibGL/GLContext.cpp b/Userland/Libraries/LibGL/GLContext.cpp index 7fa770636c..85a0f1ef5d 100644 --- a/Userland/Libraries/LibGL/GLContext.cpp +++ b/Userland/Libraries/LibGL/GLContext.cpp @@ -169,7 +169,7 @@ void GLContext::gl_end() VERIFY_NOT_REACHED(); } - m_rasterizer->draw_primitives(primitive_type, model_view_matrix(), projection_matrix(), m_vertex_list); + m_rasterizer->draw_primitives(primitive_type, m_vertex_list); m_vertex_list.clear_with_capacity(); } @@ -837,7 +837,8 @@ void GLContext::gl_raster_pos(GLfloat x, GLfloat y, GLfloat z, GLfloat w) APPEND_TO_CALL_LIST_AND_RETURN_IF_NEEDED(gl_raster_pos, x, y, z, w); RETURN_WITH_ERROR_IF(m_in_draw_state, GL_INVALID_OPERATION); - m_rasterizer->set_raster_position({ x, y, z, w }, model_view_matrix(), projection_matrix()); + sync_matrices(); + m_rasterizer->set_raster_position({ x, y, z, w }); } void GLContext::gl_line_width(GLfloat width) @@ -917,11 +918,12 @@ void GLContext::present() void GLContext::sync_device_config() { + sync_clip_planes(); sync_device_sampler_config(); sync_device_texture_units(); sync_light_state(); + sync_matrices(); sync_stencil_configuration(); - sync_clip_planes(); } ErrorOr GLContext::build_extension_string() diff --git a/Userland/Libraries/LibGL/GLContext.h b/Userland/Libraries/LibGL/GLContext.h index d7e5a27708..3f308ba270 100644 --- a/Userland/Libraries/LibGL/GLContext.h +++ b/Userland/Libraries/LibGL/GLContext.h @@ -242,12 +242,13 @@ public: void gl_get_program(GLuint program, GLenum pname, GLint* params); private: + void sync_clip_planes(); void sync_device_config(); void sync_device_sampler_config(); void sync_device_texture_units(); void sync_light_state(); + void sync_matrices(); void sync_stencil_configuration(); - void sync_clip_planes(); ErrorOr build_extension_string(); @@ -298,10 +299,12 @@ private: Vector m_model_view_matrix_stack { FloatMatrix4x4::identity() }; Vector* m_current_matrix_stack { &m_model_view_matrix_stack }; FloatMatrix4x4* m_current_matrix { &m_current_matrix_stack->last() }; + bool m_matrices_dirty { true }; ALWAYS_INLINE void update_current_matrix(FloatMatrix4x4 const& new_matrix) { *m_current_matrix = new_matrix; + m_matrices_dirty = true; if (m_current_matrix_mode == GL_TEXTURE) m_texture_units_dirty = true; diff --git a/Userland/Libraries/LibGL/Matrix.cpp b/Userland/Libraries/LibGL/Matrix.cpp index e36622a74e..2883c41edf 100644 --- a/Userland/Libraries/LibGL/Matrix.cpp +++ b/Userland/Libraries/LibGL/Matrix.cpp @@ -125,6 +125,7 @@ void GLContext::gl_pop_matrix() m_current_matrix_stack->take_last(); m_current_matrix = &m_current_matrix_stack->last(); + m_matrices_dirty = true; } void GLContext::gl_push_matrix() @@ -135,6 +136,7 @@ void GLContext::gl_push_matrix() m_current_matrix_stack->append(*m_current_matrix); m_current_matrix = &m_current_matrix_stack->last(); + m_matrices_dirty = true; } void GLContext::gl_rotate(GLfloat angle, GLfloat x, GLfloat y, GLfloat z) @@ -167,4 +169,15 @@ void GLContext::gl_translate(GLfloat x, GLfloat y, GLfloat z) update_current_matrix(*m_current_matrix * translation_matrix); } +void GLContext::sync_matrices() +{ + if (!m_matrices_dirty) + return; + + m_rasterizer->set_model_view_transform(model_view_matrix()); + m_rasterizer->set_projection_transform(projection_matrix()); + + m_matrices_dirty = false; +} + } diff --git a/Userland/Libraries/LibGPU/Device.h b/Userland/Libraries/LibGPU/Device.h index b957505c9d..ae70cd3604 100644 --- a/Userland/Libraries/LibGPU/Device.h +++ b/Userland/Libraries/LibGPU/Device.h @@ -39,7 +39,7 @@ public: virtual DeviceInfo info() const = 0; - virtual void draw_primitives(PrimitiveType, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform, Vector& vertices) = 0; + virtual void draw_primitives(PrimitiveType, Vector& vertices) = 0; virtual void resize(Gfx::IntSize min_size) = 0; virtual void clear_color(FloatVector4 const&) = 0; virtual void clear_depth(DepthType) = 0; @@ -59,6 +59,8 @@ public: virtual NonnullRefPtr create_image(PixelFormat const&, u32 width, u32 height, u32 depth, u32 max_levels) = 0; virtual ErrorOr> create_shader(IR::Shader const&) = 0; + virtual void set_model_view_transform(FloatMatrix4x4 const&) = 0; + virtual void set_projection_transform(FloatMatrix4x4 const&) = 0; virtual void set_sampler_config(unsigned, SamplerConfig const&) = 0; virtual void set_light_state(unsigned, Light const&) = 0; virtual void set_material_state(Face, Material const&) = 0; @@ -68,7 +70,7 @@ public: virtual RasterPosition raster_position() const = 0; virtual void set_raster_position(RasterPosition const& raster_position) = 0; - virtual void set_raster_position(FloatVector4 const& position, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform) = 0; + virtual void set_raster_position(FloatVector4 const& position) = 0; virtual void bind_fragment_shader(RefPtr) = 0; }; diff --git a/Userland/Libraries/LibSoftGPU/Device.cpp b/Userland/Libraries/LibSoftGPU/Device.cpp index 62364f205c..aeea528f7e 100644 --- a/Userland/Libraries/LibSoftGPU/Device.cpp +++ b/Userland/Libraries/LibSoftGPU/Device.cpp @@ -952,7 +952,7 @@ void Device::calculate_vertex_lighting(GPU::Vertex& vertex) const vertex.color.clamp(0.0f, 1.0f); } -void Device::draw_primitives(GPU::PrimitiveType primitive_type, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform, Vector& vertices) +void Device::draw_primitives(GPU::PrimitiveType primitive_type, Vector& vertices) { // At this point, the user has effectively specified that they are done with defining the geometry // of what they want to draw. We now need to do a few things (https://www.khronos.org/opengl/wiki/Rendering_Pipeline_Overview): @@ -967,21 +967,17 @@ void Device::draw_primitives(GPU::PrimitiveType primitive_type, FloatMatrix4x4 c if (vertices.is_empty()) return; - // 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 normal_transform = model_view_transform.submatrix_from_topleft<3>().transpose().inverse(); - // First, transform all vertices for (auto& vertex : vertices) { - vertex.eye_coordinates = model_view_transform * vertex.position; + vertex.eye_coordinates = m_model_view_transform * vertex.position; - vertex.normal = normal_transform * vertex.normal; + vertex.normal = m_normal_transform * vertex.normal; if (m_options.normalization_enabled) vertex.normal.normalize(); calculate_vertex_lighting(vertex); - vertex.clip_coordinates = projection_transform * vertex.eye_coordinates; + vertex.clip_coordinates = m_projection_transform * vertex.eye_coordinates; for (GPU::TextureUnitIndex i = 0; i < GPU::NUM_TEXTURE_UNITS; ++i) { auto const& texture_unit_configuration = m_texture_unit_configuration[i]; @@ -1581,6 +1577,20 @@ ErrorOr> Device::create_shader(GPU::IR::Shader const& return shader; } +void Device::set_model_view_transform(Gfx::FloatMatrix4x4 const& model_view_transform) +{ + m_model_view_transform = model_view_transform; + + // 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 + m_normal_transform = model_view_transform.submatrix_from_topleft<3>().transpose().inverse(); +} + +void Device::set_projection_transform(Gfx::FloatMatrix4x4 const& projection_transform) +{ + m_projection_transform = projection_transform; +} + void Device::set_sampler_config(unsigned sampler, GPU::SamplerConfig const& config) { VERIFY(config.bound_image.is_null() || config.bound_image->ownership_token() == this); @@ -1626,10 +1636,10 @@ void Device::set_clip_planes(Vector const& clip_planes) m_clip_planes = clip_planes; } -void Device::set_raster_position(FloatVector4 const& position, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform) +void Device::set_raster_position(FloatVector4 const& position) { - auto const eye_coordinates = model_view_transform * position; - auto const clip_coordinates = projection_transform * eye_coordinates; + auto const eye_coordinates = m_model_view_transform * position; + auto const clip_coordinates = m_projection_transform * eye_coordinates; // FIXME: implement clipping m_raster_position.valid = true; diff --git a/Userland/Libraries/LibSoftGPU/Device.h b/Userland/Libraries/LibSoftGPU/Device.h index b53e324698..582f3c6f44 100644 --- a/Userland/Libraries/LibSoftGPU/Device.h +++ b/Userland/Libraries/LibSoftGPU/Device.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -48,7 +49,7 @@ public: virtual GPU::DeviceInfo info() const override; - virtual void draw_primitives(GPU::PrimitiveType, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform, Vector& vertices) override; + virtual void draw_primitives(GPU::PrimitiveType, Vector& vertices) override; virtual void resize(Gfx::IntSize min_size) override; virtual void clear_color(FloatVector4 const&) override; virtual void clear_depth(GPU::DepthType) override; @@ -68,6 +69,8 @@ public: virtual NonnullRefPtr create_image(GPU::PixelFormat const&, u32 width, u32 height, u32 depth, u32 max_levels) override; virtual ErrorOr> create_shader(GPU::IR::Shader const&) override; + virtual void set_model_view_transform(FloatMatrix4x4 const&) override; + virtual void set_projection_transform(FloatMatrix4x4 const&) override; virtual void set_sampler_config(unsigned, GPU::SamplerConfig const&) override; virtual void set_light_state(unsigned, GPU::Light const&) override; virtual void set_material_state(GPU::Face, GPU::Material const&) override; @@ -77,7 +80,7 @@ public: virtual GPU::RasterPosition raster_position() const override { return m_raster_position; } virtual void set_raster_position(GPU::RasterPosition const& raster_position) override; - virtual void set_raster_position(FloatVector4 const& position, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform) override; + virtual void set_raster_position(FloatVector4 const& position) override; virtual void bind_fragment_shader(RefPtr) override; @@ -105,6 +108,9 @@ private: RefPtr> m_frame_buffer {}; GPU::RasterizerOptions m_options; + FloatMatrix4x4 m_model_view_transform; + FloatMatrix3x3 m_normal_transform; + FloatMatrix4x4 m_projection_transform; GPU::LightModelParameters m_lighting_model; Clipper m_clipper; Vector m_triangle_list; diff --git a/Userland/Libraries/LibVirtGPU/Device.cpp b/Userland/Libraries/LibVirtGPU/Device.cpp index 34c2bf9de8..5a229f22c2 100644 --- a/Userland/Libraries/LibVirtGPU/Device.cpp +++ b/Userland/Libraries/LibVirtGPU/Device.cpp @@ -193,7 +193,7 @@ void Device::encode_constant_buffer(Gfx::FloatMatrix4x4 const& matrix, Vector& vertices) +void Device::draw_primitives(GPU::PrimitiveType primitive_type, Vector& vertices) { // Transform incoming vertices to our own format. m_vertices.clear_with_capacity(); @@ -211,7 +211,7 @@ void Device::draw_primitives(GPU::PrimitiveType primitive_type, FloatMatrix4x4 c // Compute combined transform matrix // Flip the y axis. This is done because OpenGLs coordinate space has a Y-axis of // Opposite direction to that of LibGfx - auto combined_matrix = (Gfx::scale_matrix(FloatVector3 { 1, -1, 1 }) * projection_matrix * modelview_matrix).transpose(); + auto combined_matrix = (Gfx::scale_matrix(FloatVector3 { 1, -1, 1 }) * m_projection_transform * m_model_view_transform).transpose(); encode_constant_buffer(combined_matrix, m_constant_buffer_data); // Create command buffer @@ -372,6 +372,16 @@ ErrorOr> Device::create_shader(GPU::IR::Shader const& return adopt_ref(*new Shader(this)); } +void Device::set_model_view_transform(Gfx::FloatMatrix4x4 const& model_view_transform) +{ + m_model_view_transform = model_view_transform; +} + +void Device::set_projection_transform(Gfx::FloatMatrix4x4 const& projection_transform) +{ + m_projection_transform = projection_transform; +} + void Device::set_sampler_config(unsigned, GPU::SamplerConfig const&) { dbgln("VirtGPU::Device::set_sampler_config(): unimplemented"); @@ -413,7 +423,7 @@ void Device::set_raster_position(GPU::RasterPosition const&) dbgln("VirtGPU::Device::set_raster_position(): unimplemented"); } -void Device::set_raster_position(FloatVector4 const&, FloatMatrix4x4 const&, FloatMatrix4x4 const&) +void Device::set_raster_position(FloatVector4 const&) { dbgln("VirtGPU::Device::set_raster_position(): unimplemented"); } diff --git a/Userland/Libraries/LibVirtGPU/Device.h b/Userland/Libraries/LibVirtGPU/Device.h index 96fad57443..d6197ea477 100644 --- a/Userland/Libraries/LibVirtGPU/Device.h +++ b/Userland/Libraries/LibVirtGPU/Device.h @@ -26,7 +26,7 @@ public: virtual GPU::DeviceInfo info() const override; - virtual void draw_primitives(GPU::PrimitiveType, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform, Vector& vertices) override; + virtual void draw_primitives(GPU::PrimitiveType, Vector& vertices) override; virtual void resize(Gfx::IntSize min_size) override; virtual void clear_color(FloatVector4 const&) override; virtual void clear_depth(GPU::DepthType) override; @@ -46,6 +46,8 @@ public: virtual NonnullRefPtr create_image(GPU::PixelFormat const&, u32 width, u32 height, u32 depth, u32 max_levels) override; virtual ErrorOr> create_shader(GPU::IR::Shader const&) override; + virtual void set_model_view_transform(FloatMatrix4x4 const&) override; + virtual void set_projection_transform(FloatMatrix4x4 const&) override; virtual void set_sampler_config(unsigned, GPU::SamplerConfig const&) override; virtual void set_light_state(unsigned, GPU::Light const&) override; virtual void set_material_state(GPU::Face, GPU::Material const&) override; @@ -55,7 +57,7 @@ public: virtual GPU::RasterPosition raster_position() const override; virtual void set_raster_position(GPU::RasterPosition const& raster_position) override; - virtual void set_raster_position(FloatVector4 const& position, FloatMatrix4x4 const& model_view_transform, FloatMatrix4x4 const& projection_transform) override; + virtual void set_raster_position(FloatVector4 const& position) override; virtual void bind_fragment_shader(RefPtr) override; @@ -67,6 +69,9 @@ private: NonnullOwnPtr m_gpu_file; + FloatMatrix4x4 m_model_view_transform; + FloatMatrix4x4 m_projection_transform; + Protocol::ResourceID m_vbo_resource_id { 0 }; Protocol::ResourceID m_drawtarget { 0 }; Protocol::ResourceID m_depthbuffer_surface { 0 };