From 84c4b66721c893775938e40808486e1ce506732e Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Fri, 26 Aug 2022 15:59:51 +0200 Subject: [PATCH] LibGL+LibGPU+LibSoftGPU: Implement texture pixel format support In OpenGL this is called the (base) internal format which is an expectation expressed by the client for the minimum supported texel storage format in the GPU for textures. Since we store everything as RGBA in a `FloatVector4`, the only thing we do in this patch is remember the expected internal format, and when we write new texels we fixate the value for the alpha channel to 1 for two formats that require it. `PixelConverter` has learned how to transform pixels during transfer to support this. --- Tests/LibGL/TestRender.cpp | 29 ++++++++ .../0007_test_rgba_to_rgb_texture.qoi | Bin 0 -> 277 bytes Userland/Libraries/LibGL/Image.cpp | 67 ++++++++++++++++++ Userland/Libraries/LibGL/Image.h | 1 + Userland/Libraries/LibGL/Texture.cpp | 10 +-- Userland/Libraries/LibGPU/Device.h | 2 +- Userland/Libraries/LibGPU/Image.h | 4 +- Userland/Libraries/LibSoftGPU/Device.cpp | 16 ++--- Userland/Libraries/LibSoftGPU/Device.h | 2 +- Userland/Libraries/LibSoftGPU/Image.cpp | 27 +++++-- Userland/Libraries/LibSoftGPU/Image.h | 8 ++- .../Libraries/LibSoftGPU/PixelConverter.cpp | 4 +- .../Libraries/LibSoftGPU/PixelConverter.h | 3 +- 13 files changed, 141 insertions(+), 32 deletions(-) create mode 100644 Tests/LibGL/reference-images/0007_test_rgba_to_rgb_texture.qoi diff --git a/Tests/LibGL/TestRender.cpp b/Tests/LibGL/TestRender.cpp index 73c287cf513..e772db8f4f0 100644 --- a/Tests/LibGL/TestRender.cpp +++ b/Tests/LibGL/TestRender.cpp @@ -199,3 +199,32 @@ TEST_CASE(0006_test_rgb565_texture) context->present(); expect_bitmap_equals_reference(context->frontbuffer(), "0006_test_rgb565_texture"sv); } + +TEST_CASE(0007_test_rgba_to_rgb_texture) +{ + auto context = create_testing_context(64, 64); + + GLuint texture_id; + glGenTextures(1, &texture_id); + glBindTexture(GL_TEXTURE_2D, texture_id); + + // Write RGBA data with A = 0 to an RGB texture + u32 texture_data[] = { 0x00FF0000 }; + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, 1, 1, 0, GL_RGBA, GL_UNSIGNED_INT_8_8_8_8, texture_data); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + + glEnable(GL_TEXTURE_2D); + glBegin(GL_TRIANGLES); + glTexCoord2i(0, 0); + glVertex2i(-1, 1); + glTexCoord2i(0, 1); + glVertex2i(-1, -1); + glTexCoord2i(1, 1); + glVertex2i(1, -1); + glEnd(); + + EXPECT_EQ(glGetError(), 0u); + + context->present(); + expect_bitmap_equals_reference(context->frontbuffer(), "0007_test_rgba_to_rgb_texture"sv); +} diff --git a/Tests/LibGL/reference-images/0007_test_rgba_to_rgb_texture.qoi b/Tests/LibGL/reference-images/0007_test_rgba_to_rgb_texture.qoi new file mode 100644 index 0000000000000000000000000000000000000000..73b517c1677c2441acfe9ccae5cf4538aad79f85 GIT binary patch literal 277 zcmWmAM@~Wk007Z%;(=x}drxEH%6LOSDI&dhQE5t-B2p|&@-8rUw>Q^85L}&I2B+{( z@Edp{ get_validated_pixel_type(GLenum target, GLenum internal_format, GLenum format, GLenum type); +GPU::PixelFormat pixel_format_for_internal_format(GLenum internal_format); } diff --git a/Userland/Libraries/LibGL/Texture.cpp b/Userland/Libraries/LibGL/Texture.cpp index 4e80a7bff68..f6afb81bd17 100644 --- a/Userland/Libraries/LibGL/Texture.cpp +++ b/Userland/Libraries/LibGL/Texture.cpp @@ -308,13 +308,6 @@ void GLContext::gl_tex_gen_floatv(GLenum coord, GLenum pname, GLfloat const* par m_texcoord_generation_dirty = true; } -// FIXME: talk to GPU::Device to determine supported GPU::PixelTypes -constexpr GPU::PixelType texture_fixed_pixel_type = { - .format = GPU::PixelFormat::RGBA, - .bits = GPU::PixelComponentBits::AllBits, - .data_type = GPU::PixelDataType::Float, -}; - void GLContext::gl_tex_image_2d(GLenum target, GLint level, GLint internal_format, GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type, GLvoid const* data) { RETURN_WITH_ERROR_IF(m_in_draw_state, GL_INVALID_OPERATION); @@ -341,7 +334,8 @@ void GLContext::gl_tex_image_2d(GLenum target, GLint level, GLint internal_forma // that constructing GL textures in any but the default mipmap order, going from level 0 upwards will cause mip levels to stay uninitialized. // To be spec compliant we should create the device image once the texture has become complete and is used for rendering the first time. // All images that were attached before the device image was created need to be stored somewhere to be used to initialize the device image once complete. - texture_2d->set_device_image(m_rasterizer->create_image(texture_fixed_pixel_type, width, height, 1, 999, 1)); + auto internal_pixel_format = pixel_format_for_internal_format(internal_format); + texture_2d->set_device_image(m_rasterizer->create_image(internal_pixel_format, width, height, 1, 999, 1)); m_sampler_config_is_dirty = true; } diff --git a/Userland/Libraries/LibGPU/Device.h b/Userland/Libraries/LibGPU/Device.h index 5f4ef2a6910..712bc1338aa 100644 --- a/Userland/Libraries/LibGPU/Device.h +++ b/Userland/Libraries/LibGPU/Device.h @@ -57,7 +57,7 @@ public: virtual RasterizerOptions options() const = 0; virtual LightModelParameters light_model() const = 0; - virtual NonnullRefPtr create_image(PixelType const&, u32 width, u32 height, u32 depth, u32 levels, u32 layers) = 0; + virtual NonnullRefPtr create_image(PixelFormat const&, u32 width, u32 height, u32 depth, u32 levels, u32 layers) = 0; virtual void set_sampler_config(unsigned, SamplerConfig const&) = 0; virtual void set_light_state(unsigned, Light const&) = 0; diff --git a/Userland/Libraries/LibGPU/Image.h b/Userland/Libraries/LibGPU/Image.h index 1f2e189c39c..6c882f59b0d 100644 --- a/Userland/Libraries/LibGPU/Image.h +++ b/Userland/Libraries/LibGPU/Image.h @@ -21,8 +21,8 @@ public: virtual ~Image() { } - virtual void write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* data, ImageDataLayout const&) = 0; - virtual void read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* data, ImageDataLayout const&) const = 0; + virtual void write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* input_data, ImageDataLayout const&) = 0; + virtual void read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* output_data, ImageDataLayout const&) const = 0; virtual void copy_texels(Image const& source, u32 source_layer, u32 source_level, Vector3 const& source_offset, Vector3 const& size, u32 destination_layer, u32 destination_level, Vector3 const& destination_offset) = 0; void const* ownership_token() const { return m_ownership_token; } diff --git a/Userland/Libraries/LibSoftGPU/Device.cpp b/Userland/Libraries/LibSoftGPU/Device.cpp index 5235956022e..13b1297137d 100644 --- a/Userland/Libraries/LibSoftGPU/Device.cpp +++ b/Userland/Libraries/LibSoftGPU/Device.cpp @@ -1399,7 +1399,7 @@ void Device::blit_from_color_buffer(void* output_data, Vector2 input_offset PixelConverter converter { input_layout, output_layout }; auto const* input_data = m_frame_buffer->color_buffer()->scanline(0); - auto conversion_result = converter.convert(input_data, output_data); + auto conversion_result = converter.convert(input_data, output_data, {}); if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } @@ -1411,7 +1411,7 @@ void Device::blit_from_depth_buffer(void* output_data, Vector2 input_offset PixelConverter converter { input_layout, output_layout }; auto const* input_data = m_frame_buffer->depth_buffer()->scanline(0); - auto conversion_result = converter.convert(input_data, output_data); + auto conversion_result = converter.convert(input_data, output_data, {}); if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } @@ -1432,7 +1432,7 @@ void Device::blit_to_color_buffer_at_raster_position(void const* input_data, GPU PixelConverter converter { input_layout, output_layout }; auto* output_data = m_frame_buffer->color_buffer()->scanline(0); - auto conversion_result = converter.convert(input_data, output_data); + auto conversion_result = converter.convert(input_data, output_data, {}); if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } @@ -1450,7 +1450,7 @@ void Device::blit_to_depth_buffer_at_raster_position(void const* input_data, GPU PixelConverter converter { input_layout, output_layout }; auto* output_data = m_frame_buffer->depth_buffer()->scanline(0); - auto conversion_result = converter.convert(input_data, output_data); + auto conversion_result = converter.convert(input_data, output_data, {}); if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } @@ -1533,19 +1533,15 @@ void Device::set_light_model_params(GPU::LightModelParameters const& lighting_mo m_lighting_model = lighting_model; } -NonnullRefPtr Device::create_image(GPU::PixelType const& pixel_type, u32 width, u32 height, u32 depth, u32 levels, u32 layers) +NonnullRefPtr Device::create_image(GPU::PixelFormat const& pixel_format, u32 width, u32 height, u32 depth, u32 levels, u32 layers) { - VERIFY(pixel_type.format == GPU::PixelFormat::RGBA - && pixel_type.bits == GPU::PixelComponentBits::AllBits - && pixel_type.data_type == GPU::PixelDataType::Float - && pixel_type.components_order == GPU::ComponentsOrder::Normal); VERIFY(width > 0); VERIFY(height > 0); VERIFY(depth > 0); VERIFY(levels > 0); VERIFY(layers > 0); - return adopt_ref(*new Image(this, width, height, depth, levels, layers)); + return adopt_ref(*new Image(this, pixel_format, width, height, depth, levels, layers)); } void Device::set_sampler_config(unsigned sampler, GPU::SamplerConfig const& config) diff --git a/Userland/Libraries/LibSoftGPU/Device.h b/Userland/Libraries/LibSoftGPU/Device.h index e10b4c9f321..18b4ba1c86e 100644 --- a/Userland/Libraries/LibSoftGPU/Device.h +++ b/Userland/Libraries/LibSoftGPU/Device.h @@ -62,7 +62,7 @@ public: virtual GPU::RasterizerOptions options() const override { return m_options; } virtual GPU::LightModelParameters light_model() const override { return m_lighting_model; } - virtual NonnullRefPtr create_image(GPU::PixelType const&, u32 width, u32 height, u32 depth, u32 levels, u32 layers) override; + virtual NonnullRefPtr create_image(GPU::PixelFormat const&, u32 width, u32 height, u32 depth, u32 levels, u32 layers) override; virtual void set_sampler_config(unsigned, GPU::SamplerConfig const&) override; virtual void set_light_state(unsigned, GPU::Light const&) override; diff --git a/Userland/Libraries/LibSoftGPU/Image.cpp b/Userland/Libraries/LibSoftGPU/Image.cpp index 8128e355632..9332021c483 100644 --- a/Userland/Libraries/LibSoftGPU/Image.cpp +++ b/Userland/Libraries/LibSoftGPU/Image.cpp @@ -10,11 +10,18 @@ namespace SoftGPU { -Image::Image(void const* ownership_token, u32 width, u32 height, u32 depth, u32 max_levels, u32 layers) +Image::Image(void const* ownership_token, GPU::PixelFormat const& pixel_format, u32 width, u32 height, u32 depth, u32 max_levels, u32 layers) : GPU::Image(ownership_token) , m_num_layers(layers) + , m_pixel_format(pixel_format) , m_mipmap_buffers(FixedArray>>::must_create_but_fixme_should_propagate_errors(layers * max_levels)) { + VERIFY(pixel_format == GPU::PixelFormat::Alpha + || pixel_format == GPU::PixelFormat::Intensity + || pixel_format == GPU::PixelFormat::Luminance + || pixel_format == GPU::PixelFormat::LuminanceAlpha + || pixel_format == GPU::PixelFormat::RGB + || pixel_format == GPU::PixelFormat::RGBA); VERIFY(width > 0); VERIFY(height > 0); VERIFY(depth > 0); @@ -70,20 +77,30 @@ GPU::ImageDataLayout Image::image_data_layout(u32 level, Vector3 offset) co }; } -void Image::write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* data, GPU::ImageDataLayout const& input_layout) +void Image::write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* input_data, GPU::ImageDataLayout const& input_layout) { VERIFY(layer < num_layers()); VERIFY(level < num_levels()); auto output_layout = image_data_layout(level, output_offset); + auto texel_data = texel_pointer(layer, level, 0, 0, 0); PixelConverter converter { input_layout, output_layout }; - auto conversion_result = converter.convert(data, texel_pointer(layer, level, 0, 0, 0)); + ErrorOr conversion_result; + switch (m_pixel_format) { + case GPU::PixelFormat::Luminance: + case GPU::PixelFormat::RGB: + // Both Luminance and RGB set the alpha to 1, regardless of the source texel + conversion_result = converter.convert(input_data, texel_data, [](auto& components) { components[3] = 1.f; }); + break; + default: + conversion_result = converter.convert(input_data, texel_data, {}); + } if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } -void Image::read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* data, GPU::ImageDataLayout const& output_layout) const +void Image::read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* output_data, GPU::ImageDataLayout const& output_layout) const { VERIFY(layer < num_layers()); VERIFY(level < num_levels()); @@ -91,7 +108,7 @@ void Image::read_texels(u32 layer, u32 level, Vector3 const& input_offset, auto input_layout = image_data_layout(level, input_offset); PixelConverter converter { input_layout, output_layout }; - auto conversion_result = converter.convert(texel_pointer(layer, level, 0, 0, 0), data); + auto conversion_result = converter.convert(texel_pointer(layer, level, 0, 0, 0), output_data, {}); if (conversion_result.is_error()) dbgln("Pixel conversion failed: {}", conversion_result.error().string_literal()); } diff --git a/Userland/Libraries/LibSoftGPU/Image.h b/Userland/Libraries/LibSoftGPU/Image.h index 23accd85428..4ccfc3cbd46 100644 --- a/Userland/Libraries/LibSoftGPU/Image.h +++ b/Userland/Libraries/LibSoftGPU/Image.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -19,7 +20,7 @@ namespace SoftGPU { class Image final : public GPU::Image { public: - Image(void const* ownership_token, u32 width, u32 height, u32 depth, u32 max_levels, u32 layers); + Image(void const* ownership_token, GPU::PixelFormat const&, u32 width, u32 height, u32 depth, u32 max_levels, u32 layers); u32 level_width(u32 level) const { return m_mipmap_buffers[level]->width(); } u32 level_height(u32 level) const { return m_mipmap_buffers[level]->height(); } @@ -40,8 +41,8 @@ public: *texel_pointer(layer, level, x, y, z) = color; } - virtual void write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* data, GPU::ImageDataLayout const&) override; - virtual void read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* data, GPU::ImageDataLayout const&) const override; + virtual void write_texels(u32 layer, u32 level, Vector3 const& output_offset, void const* input_data, GPU::ImageDataLayout const&) override; + virtual void read_texels(u32 layer, u32 level, Vector3 const& input_offset, void* output_data, GPU::ImageDataLayout const&) const override; virtual void copy_texels(GPU::Image const& source, u32 source_layer, u32 source_level, Vector3 const& source_offset, Vector3 const& size, u32 destination_layer, u32 destination_level, Vector3 const& destination_offset) override; private: @@ -61,6 +62,7 @@ private: u32 m_num_levels { 0 }; u32 m_num_layers { 0 }; + GPU::PixelFormat m_pixel_format; FixedArray>> m_mipmap_buffers; bool m_width_is_power_of_two { false }; diff --git a/Userland/Libraries/LibSoftGPU/PixelConverter.cpp b/Userland/Libraries/LibSoftGPU/PixelConverter.cpp index a6007de5dd2..74a8409d04e 100644 --- a/Userland/Libraries/LibSoftGPU/PixelConverter.cpp +++ b/Userland/Libraries/LibSoftGPU/PixelConverter.cpp @@ -359,7 +359,7 @@ static constexpr GPU::ImageSelection restrain_selection_within_dimensions(GPU::I return selection; } -ErrorOr PixelConverter::convert(void const* input_data, void* output_data) +ErrorOr PixelConverter::convert(void const* input_data, void* output_data, Function transform) { // Verify pixel data specifications auto validate_image_data_layout = [](GPU::ImageDataLayout const& specification) -> ErrorOr { @@ -428,6 +428,8 @@ ErrorOr PixelConverter::convert(void const* input_data, void* output_data) + output_selection.offset_x * output_pixel_size_in_bytes]; for (u32 input_x = input_selection.offset_x; input_x < input_selection.offset_x + input_selection.width; ++input_x) { auto pixel_components = read_pixel(&input_scanline); + if (transform) + transform(pixel_components); write_pixel(&output_scanline, pixel_components); } ++output_y; diff --git a/Userland/Libraries/LibSoftGPU/PixelConverter.h b/Userland/Libraries/LibSoftGPU/PixelConverter.h index 5c51c47b4b4..54492b26ba7 100644 --- a/Userland/Libraries/LibSoftGPU/PixelConverter.h +++ b/Userland/Libraries/LibSoftGPU/PixelConverter.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include @@ -20,7 +21,7 @@ public: { } - ErrorOr convert(void const* input_data, void* output_data); + ErrorOr convert(void const* input_data, void* output_data, Function transform); private: FloatVector4 read_pixel(u8 const**);