From c21a3b3029587961d39d2da95c7c43074e7e6076 Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Tue, 8 Mar 2022 12:16:56 +0100 Subject: [PATCH] LibGL: Better handling of texture targets and default textures We were lacking support for default textures (i.e. calling `glBindTexture` with a `texture` argument of `0`) which caused our Quake2 port to render red screens whenever a video was playing. Every texture unit is now initialized with a default 2D texture. Additionally, we had this concept of a "currently bound target" on our texture units which is not how OpenGL wants us to handle targets. Calling `glBindTexture` should set the texture for the provided target only, making it sort of an alias for future operations on the same target. Finally, `glDeleteTextures` should not remove the bound texture from the target in the texture unit, but it should reset it to the default texture. --- Userland/Libraries/LibGL/CMakeLists.txt | 1 - Userland/Libraries/LibGL/GL/gl.h | 2 + .../Libraries/LibGL/SoftwareGLContext.cpp | 227 +++++++++--------- Userland/Libraries/LibGL/SoftwareGLContext.h | 9 + Userland/Libraries/LibGL/Tex/Texture2D.cpp | 6 +- Userland/Libraries/LibGL/Tex/TextureUnit.cpp | 32 --- Userland/Libraries/LibGL/Tex/TextureUnit.h | 19 +- 7 files changed, 140 insertions(+), 156 deletions(-) delete mode 100644 Userland/Libraries/LibGL/Tex/TextureUnit.cpp diff --git a/Userland/Libraries/LibGL/CMakeLists.txt b/Userland/Libraries/LibGL/CMakeLists.txt index 29b7b61e44..c56c7dd405 100644 --- a/Userland/Libraries/LibGL/CMakeLists.txt +++ b/Userland/Libraries/LibGL/CMakeLists.txt @@ -16,7 +16,6 @@ set(SOURCES SoftwareGLContext.cpp Tex/NameAllocator.cpp Tex/Texture2D.cpp - Tex/TextureUnit.cpp ) serenity_lib(LibGL gl) diff --git a/Userland/Libraries/LibGL/GL/gl.h b/Userland/Libraries/LibGL/GL/gl.h index 6aba866260..e8da547143 100644 --- a/Userland/Libraries/LibGL/GL/gl.h +++ b/Userland/Libraries/LibGL/GL/gl.h @@ -339,6 +339,8 @@ extern "C" { #define GL_TEXTURE_3D 0x806F #define GL_PROXY_TEXTURE_3D 0x8070 #define GL_TEXTURE_CUBE_MAP 0x8513 +#define GL_TEXTURE_1D_ARRAY 0x8C18 +#define GL_TEXTURE_2D_ARRAY 0x8C1A // Texture parameters #define GL_TEXTURE_WIDTH 0x1000 diff --git a/Userland/Libraries/LibGL/SoftwareGLContext.cpp b/Userland/Libraries/LibGL/SoftwareGLContext.cpp index 792e85ebfa..8ca2f49d57 100644 --- a/Userland/Libraries/LibGL/SoftwareGLContext.cpp +++ b/Userland/Libraries/LibGL/SoftwareGLContext.cpp @@ -68,6 +68,13 @@ SoftwareGLContext::SoftwareGLContext(Gfx::Bitmap& frontbuffer) m_texture_units.resize(m_device_info.num_texture_units); m_active_texture_unit = &m_texture_units[0]; + // All texture units are initialized with default textures for all targets; these + // can be referenced later on with texture name 0 in operations like glBindTexture(). + auto default_texture_2d = adopt_ref(*new Texture2D()); + m_default_textures.set(GL_TEXTURE_2D, default_texture_2d); + for (auto& texture_unit : m_texture_units) + texture_unit.set_texture_2d_target_texture(default_texture_2d); + // Query the number lights from the device and set set up their state // locally in the GL m_light_states.resize(m_device_info.num_lights); @@ -914,9 +921,8 @@ void SoftwareGLContext::gl_gen_textures(GLsizei n, GLuint* textures) m_name_allocator.allocate(n, textures); // Initialize all texture names with a nullptr - for (auto i = 0; i < n; i++) { + for (auto i = 0; i < n; ++i) { GLuint name = textures[i]; - m_allocated_textures.set(name, nullptr); } } @@ -936,11 +942,14 @@ void SoftwareGLContext::gl_delete_textures(GLsizei n, const GLuint* textures) auto texture_object = m_allocated_textures.find(name); if (texture_object == m_allocated_textures.end() || texture_object->value.is_null()) continue; + auto texture = texture_object->value; // Check all texture units for (auto& texture_unit : m_texture_units) { - if (texture_object->value == texture_unit.bound_texture()) - texture_unit.bind_texture_to_target(GL_TEXTURE_2D, nullptr); + if (texture->is_texture_2d() && texture_unit.texture_2d_target_texture() == texture) { + // If a texture that is currently bound is deleted, the binding reverts to 0 (the default texture) + texture_unit.set_texture_2d_target_texture(get_default_texture(GL_TEXTURE_2D)); + } } m_allocated_textures.remove(name); @@ -954,9 +963,6 @@ void SoftwareGLContext::gl_tex_image_2d(GLenum target, GLint level, GLint intern // We only support GL_TEXTURE_2D for now RETURN_WITH_ERROR_IF(target != GL_TEXTURE_2D, GL_INVALID_ENUM); - // Check if there is actually a texture bound - RETURN_WITH_ERROR_IF(target == GL_TEXTURE_2D && m_active_texture_unit->currently_bound_target() != GL_TEXTURE_2D, GL_INVALID_OPERATION); - // Internal format can also be a number between 1 and 4. Symbolic formats were only added with EXT_texture, promoted to core in OpenGL 1.1 if (internal_format == 1) internal_format = GL_ALPHA; @@ -979,6 +985,9 @@ void SoftwareGLContext::gl_tex_image_2d(GLenum target, GLint level, GLint intern } RETURN_WITH_ERROR_IF(border != 0, GL_INVALID_VALUE); + auto texture_2d = m_active_texture_unit->texture_2d_target_texture(); + VERIFY(!texture_2d.is_null()); + if (level == 0) { // FIXME: OpenGL has the concept of texture and mipmap completeness. A texture has to fulfill certain criteria to be considered complete. // Trying to render while an incomplete texture is bound will result in an error. @@ -986,11 +995,11 @@ void SoftwareGLContext::gl_tex_image_2d(GLenum target, GLint level, GLint intern // 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. - m_active_texture_unit->bound_texture_2d()->set_device_image(m_rasterizer.create_image(SoftGPU::ImageFormat::BGRA8888, width, height, 1, 999, 1)); + texture_2d->set_device_image(m_rasterizer.create_image(SoftGPU::ImageFormat::BGRA8888, width, height, 1, 999, 1)); m_sampler_config_is_dirty = true; } - m_active_texture_unit->bound_texture_2d()->upload_texture_data(level, internal_format, width, height, format, type, data, m_unpack_row_length, m_unpack_alignment); + texture_2d->upload_texture_data(level, internal_format, width, height, format, type, data, m_unpack_row_length, m_unpack_alignment); } void SoftwareGLContext::gl_tex_sub_image_2d(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLenum type, GLvoid const* data) @@ -1000,20 +1009,19 @@ void SoftwareGLContext::gl_tex_sub_image_2d(GLenum target, GLint level, GLint xo // We only support GL_TEXTURE_2D for now RETURN_WITH_ERROR_IF(target != GL_TEXTURE_2D, GL_INVALID_ENUM); - // Check if there is actually a texture bound - RETURN_WITH_ERROR_IF(target == GL_TEXTURE_2D && m_active_texture_unit->currently_bound_target() != GL_TEXTURE_2D, GL_INVALID_OPERATION); - // We only support symbolic constants for now RETURN_WITH_ERROR_IF(!(format == GL_RGBA || format == GL_RGB), GL_INVALID_VALUE); RETURN_WITH_ERROR_IF(!(type == GL_UNSIGNED_BYTE || type == GL_UNSIGNED_SHORT_5_6_5), GL_INVALID_VALUE); RETURN_WITH_ERROR_IF(level < 0 || level > Texture2D::LOG2_MAX_TEXTURE_SIZE, GL_INVALID_VALUE); RETURN_WITH_ERROR_IF(width < 0 || height < 0 || width > (2 + Texture2D::MAX_TEXTURE_SIZE) || height > (2 + Texture2D::MAX_TEXTURE_SIZE), GL_INVALID_VALUE); - auto texture = m_active_texture_unit->bound_texture_2d(); + // A 2D texture array must have been defined by a previous glTexImage2D operation + auto texture_2d = m_active_texture_unit->texture_2d_target_texture(); + RETURN_WITH_ERROR_IF(texture_2d.is_null(), GL_INVALID_OPERATION); - RETURN_WITH_ERROR_IF(xoffset < 0 || yoffset < 0 || xoffset + width > texture->width_at_lod(level) || yoffset + height > texture->height_at_lod(level), GL_INVALID_VALUE); + RETURN_WITH_ERROR_IF(xoffset < 0 || yoffset < 0 || xoffset + width > texture_2d->width_at_lod(level) || yoffset + height > texture_2d->height_at_lod(level), GL_INVALID_VALUE); - texture->replace_sub_texture_data(level, xoffset, yoffset, width, height, format, type, data, m_unpack_row_length, m_unpack_alignment); + texture_2d->replace_sub_texture_data(level, xoffset, yoffset, width, height, format, type, data, m_unpack_row_length, m_unpack_alignment); } void SoftwareGLContext::gl_tex_parameter(GLenum target, GLenum pname, GLfloat param) @@ -1032,57 +1040,56 @@ void SoftwareGLContext::gl_tex_parameter(GLenum target, GLenum pname, GLfloat pa || pname == GL_TEXTURE_WRAP_T), GL_INVALID_ENUM); - if (target == GL_TEXTURE_2D) { - auto texture2d = m_active_texture_unit->bound_texture_2d(); - if (texture2d.is_null()) - return; + // We assume GL_TEXTURE_2D (see above) + auto texture_2d = m_active_texture_unit->texture_2d_target_texture(); + if (texture_2d.is_null()) + return; - switch (pname) { - case GL_TEXTURE_MIN_FILTER: - RETURN_WITH_ERROR_IF(!(param == GL_NEAREST - || param == GL_LINEAR - || param == GL_NEAREST_MIPMAP_NEAREST - || param == GL_LINEAR_MIPMAP_NEAREST - || param == GL_NEAREST_MIPMAP_LINEAR - || param == GL_LINEAR_MIPMAP_LINEAR), - GL_INVALID_ENUM); + switch (pname) { + case GL_TEXTURE_MIN_FILTER: + RETURN_WITH_ERROR_IF(!(param == GL_NEAREST + || param == GL_LINEAR + || param == GL_NEAREST_MIPMAP_NEAREST + || param == GL_LINEAR_MIPMAP_NEAREST + || param == GL_NEAREST_MIPMAP_LINEAR + || param == GL_LINEAR_MIPMAP_LINEAR), + GL_INVALID_ENUM); - texture2d->sampler().set_min_filter(param); - break; + texture_2d->sampler().set_min_filter(param); + break; - case GL_TEXTURE_MAG_FILTER: - RETURN_WITH_ERROR_IF(!(param == GL_NEAREST - || param == GL_LINEAR), - GL_INVALID_ENUM); + case GL_TEXTURE_MAG_FILTER: + RETURN_WITH_ERROR_IF(!(param == GL_NEAREST + || param == GL_LINEAR), + GL_INVALID_ENUM); - texture2d->sampler().set_mag_filter(param); - break; + texture_2d->sampler().set_mag_filter(param); + break; - case GL_TEXTURE_WRAP_S: - RETURN_WITH_ERROR_IF(!(param == GL_CLAMP - || param == GL_CLAMP_TO_BORDER - || param == GL_CLAMP_TO_EDGE - || param == GL_MIRRORED_REPEAT - || param == GL_REPEAT), - GL_INVALID_ENUM); + case GL_TEXTURE_WRAP_S: + RETURN_WITH_ERROR_IF(!(param == GL_CLAMP + || param == GL_CLAMP_TO_BORDER + || param == GL_CLAMP_TO_EDGE + || param == GL_MIRRORED_REPEAT + || param == GL_REPEAT), + GL_INVALID_ENUM); - texture2d->sampler().set_wrap_s_mode(param); - break; + texture_2d->sampler().set_wrap_s_mode(param); + break; - case GL_TEXTURE_WRAP_T: - RETURN_WITH_ERROR_IF(!(param == GL_CLAMP - || param == GL_CLAMP_TO_BORDER - || param == GL_CLAMP_TO_EDGE - || param == GL_MIRRORED_REPEAT - || param == GL_REPEAT), - GL_INVALID_ENUM); + case GL_TEXTURE_WRAP_T: + RETURN_WITH_ERROR_IF(!(param == GL_CLAMP + || param == GL_CLAMP_TO_BORDER + || param == GL_CLAMP_TO_EDGE + || param == GL_MIRRORED_REPEAT + || param == GL_REPEAT), + GL_INVALID_ENUM); - texture2d->sampler().set_wrap_t_mode(param); - break; + texture_2d->sampler().set_wrap_t_mode(param); + break; - default: - VERIFY_NOT_REACHED(); - } + default: + VERIFY_NOT_REACHED(); } m_sampler_config_is_dirty = true; @@ -1810,55 +1817,49 @@ void SoftwareGLContext::gl_read_pixels(GLint x, GLint y, GLsizei width, GLsizei void SoftwareGLContext::gl_bind_texture(GLenum target, GLuint texture) { RETURN_WITH_ERROR_IF(m_in_draw_state, GL_INVALID_OPERATION); + RETURN_WITH_ERROR_IF(target != GL_TEXTURE_1D + && target != GL_TEXTURE_2D + && target != GL_TEXTURE_3D + && target != GL_TEXTURE_1D_ARRAY + && target != GL_TEXTURE_2D_ARRAY + && target != GL_TEXTURE_CUBE_MAP, + GL_INVALID_ENUM); + // FIXME: We only support GL_TEXTURE_2D for now - RETURN_WITH_ERROR_IF(target != GL_TEXTURE_2D, GL_INVALID_ENUM); + if (target != GL_TEXTURE_2D) { + dbgln("gl_bind_texture(target = {:#x}): currently only GL_TEXTURE_2D is supported", target); + return; + } + + RefPtr texture_2d; if (texture == 0) { - switch (target) { - case GL_TEXTURE_2D: - m_active_texture_unit->bind_texture_to_target(target, nullptr); - m_sampler_config_is_dirty = true; - return; - default: - VERIFY_NOT_REACHED(); - return; + // Texture name 0 refers to the default texture + texture_2d = get_default_texture(target); + } else { + // Find this texture name in our previously allocated textures + auto it = m_allocated_textures.find(texture); + if (it != m_allocated_textures.end()) { + auto texture_object = it->value; + if (!texture_object.is_null()) { + // Texture must have been created with the same target + RETURN_WITH_ERROR_IF(!texture_object->is_texture_2d(), GL_INVALID_OPERATION); + texture_2d = static_cast(texture_object.ptr()); + } + } + + // OpenGL 1.x supports binding texture names that were not previously generated by glGenTextures. + // If there is not an allocated texture, meaning it was not previously generated by glGenTextures, + // we can keep texture_object null to both allocate and bind the texture with the passed in texture name. + // FIXME: Later OpenGL versions such as 4.x enforce that texture names being bound were previously generated + // by glGenTextures. + if (!texture_2d) { + texture_2d = adopt_ref(*new Texture2D()); + m_allocated_textures.set(texture, texture_2d); } } - auto it = m_allocated_textures.find(texture); - RefPtr texture_object; - - // OpenGL 1.x supports binding texture names that were not previously generated by glGenTextures. - // If there is not an allocated texture, meaning it was not previously generated by glGenTextures, - // we can keep texture_object null to both allocate and bind the texture with the passed in texture name. - // FIXME: Later OpenGL versions such as 4.x enforce that texture names being bound were previously generated - // by glGenTextures. - if (it != m_allocated_textures.end()) - texture_object = it->value; - - // Binding a texture to a different target than it was first bound is an invalid operation - // FIXME: We only support GL_TEXTURE_2D for now - RETURN_WITH_ERROR_IF(target == GL_TEXTURE_2D && !texture_object.is_null() && !texture_object->is_texture_2d(), GL_INVALID_OPERATION); - - if (!texture_object) { - // This is the first time the texture is bound. Allocate an actual texture object - switch (target) { - case GL_TEXTURE_2D: - texture_object = adopt_ref(*new Texture2D()); - break; - default: - VERIFY_NOT_REACHED(); - } - - m_allocated_textures.set(texture, texture_object); - } - - switch (target) { - case GL_TEXTURE_2D: - m_active_texture_unit->bind_texture_to_target(target, texture_object); - break; - } - + m_active_texture_unit->set_texture_2d_target_texture(texture_2d); m_sampler_config_is_dirty = true; } @@ -2850,12 +2851,15 @@ void SoftwareGLContext::gl_get_tex_parameter_integerv(GLenum target, GLint level // FIXME: GL_INVALID_VALUE is generated if target is GL_TEXTURE_BUFFER and level is not zero // FIXME: GL_INVALID_OPERATION is generated if GL_TEXTURE_COMPRESSED_IMAGE_SIZE is queried on texture images with an uncompressed internal format or on proxy targets + VERIFY(!m_active_texture_unit->texture_2d_target_texture().is_null()); + auto const texture_2d = m_active_texture_unit->texture_2d_target_texture(); + switch (pname) { case GL_TEXTURE_HEIGHT: - *params = m_active_texture_unit->bound_texture_2d()->height_at_lod(level); + *params = texture_2d->height_at_lod(level); break; case GL_TEXTURE_WIDTH: - *params = m_active_texture_unit->bound_texture_2d()->width_at_lod(level); + *params = texture_2d->width_at_lod(level); break; } } @@ -2966,16 +2970,23 @@ void SoftwareGLContext::sync_device_sampler_config() m_sampler_config_is_dirty = false; for (unsigned i = 0; i < m_texture_units.size(); ++i) { - SoftGPU::SamplerConfig config; + auto const& texture_unit = m_texture_units[i]; - if (!m_texture_units[i].texture_2d_enabled()) + if (!texture_unit.texture_2d_enabled()) continue; - auto texture = m_texture_units[i].bound_texture_2d(); + SoftGPU::SamplerConfig config; - config.bound_image = texture.is_null() ? nullptr : texture->device_image(); + auto texture_2d = texture_unit.texture_2d_target_texture(); + if (texture_2d.is_null()) { + config.bound_image = nullptr; + m_rasterizer.set_sampler_config(i, config); + continue; + } - auto const& sampler = texture->sampler(); + config.bound_image = texture_2d->device_image(); + + auto const& sampler = texture_2d->sampler(); switch (sampler.min_filter()) { case GL_NEAREST: @@ -3057,7 +3068,7 @@ void SoftwareGLContext::sync_device_sampler_config() VERIFY_NOT_REACHED(); } - switch (m_texture_units[i].env_mode()) { + switch (texture_unit.env_mode()) { case GL_MODULATE: config.fixed_function_texture_env_mode = SoftGPU::TextureEnvMode::Modulate; break; diff --git a/Userland/Libraries/LibGL/SoftwareGLContext.h b/Userland/Libraries/LibGL/SoftwareGLContext.h index 1b4cab1bce..f88731f782 100644 --- a/Userland/Libraries/LibGL/SoftwareGLContext.h +++ b/Userland/Libraries/LibGL/SoftwareGLContext.h @@ -270,8 +270,17 @@ private: NonnullRefPtr m_frontbuffer; // Texture objects + template + RefPtr get_default_texture(GLenum target) + { + auto default_texture = m_default_textures.get(target); + VERIFY(default_texture.has_value()); + return static_cast(default_texture.value()); + } + TextureNameAllocator m_name_allocator; HashMap> m_allocated_textures; + HashMap> m_default_textures; Vector m_texture_units; TextureUnit* m_active_texture_unit; size_t m_active_texture_unit_index { 0 }; diff --git a/Userland/Libraries/LibGL/Tex/Texture2D.cpp b/Userland/Libraries/LibGL/Tex/Texture2D.cpp index b12f307340..dd33c3a104 100644 --- a/Userland/Libraries/LibGL/Tex/Texture2D.cpp +++ b/Userland/Libraries/LibGL/Tex/Texture2D.cpp @@ -20,12 +20,12 @@ void Texture2D::upload_texture_data(GLuint lod, GLint internal_format, GLsizei w mip.set_width(width); mip.set_height(height); - // No pixel data was supplied leave the texture memory uninitialized. + m_internal_format = internal_format; + + // No pixel data was supplied; leave the texture memory uninitialized. if (pixels == nullptr) return; - m_internal_format = internal_format; - replace_sub_texture_data(lod, 0, 0, width, height, format, type, pixels, pixels_per_row, byte_alignment); } diff --git a/Userland/Libraries/LibGL/Tex/TextureUnit.cpp b/Userland/Libraries/LibGL/Tex/TextureUnit.cpp deleted file mode 100644 index 852980d339..0000000000 --- a/Userland/Libraries/LibGL/Tex/TextureUnit.cpp +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (c) 2021, Jesse Buhagiar - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include - -namespace GL { - -void TextureUnit::bind_texture_to_target(GLenum texture_target, const RefPtr& texture) -{ - if (!texture) { - m_texture_target_2d = nullptr; - m_currently_bound_target = GL_NONE; - m_currently_bound_texture = nullptr; - return; - } - - switch (texture_target) { - case GL_TEXTURE_2D: - m_texture_target_2d = static_ptr_cast(texture); - m_currently_bound_target = GL_TEXTURE_2D; - m_currently_bound_texture = texture; - break; - default: - VERIFY_NOT_REACHED(); - } -} - -} diff --git a/Userland/Libraries/LibGL/Tex/TextureUnit.h b/Userland/Libraries/LibGL/Tex/TextureUnit.h index f7ec122047..c485ca9c91 100644 --- a/Userland/Libraries/LibGL/Tex/TextureUnit.h +++ b/Userland/Libraries/LibGL/Tex/TextureUnit.h @@ -1,13 +1,13 @@ /* * Copyright (c) 2021, Jesse Buhagiar + * Copyright (c) 2022, Jelle Raaijmakers * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once -#include -#include +#include #include namespace GL { @@ -16,13 +16,8 @@ class TextureUnit { public: TextureUnit() = default; - void bind_texture_to_target(GLenum texture_target, const RefPtr& texture); - - RefPtr& bound_texture_2d() const { return m_texture_target_2d; } - RefPtr& bound_texture() const { return m_currently_bound_texture; } - - GLenum currently_bound_target() const { return m_currently_bound_target; } - bool is_bound() const { return !m_currently_bound_texture.is_null(); } + RefPtr texture_2d_target_texture() const { return m_texture_2d_target_texture; } + void set_texture_2d_target_texture(RefPtr const& texture) { m_texture_2d_target_texture = texture; } void set_env_mode(GLenum mode) { m_env_mode = mode; } GLenum env_mode() const { return m_env_mode; } @@ -37,11 +32,11 @@ public: void set_texture_cube_map_enabled(bool texture_cube_map_enabled) { m_texture_cube_map_enabled = texture_cube_map_enabled; } private: - mutable RefPtr m_texture_target_2d { nullptr }; - mutable RefPtr m_currently_bound_texture { nullptr }; - GLenum m_currently_bound_target; GLenum m_env_mode { GL_MODULATE }; + // Bound textures + RefPtr m_texture_2d_target_texture {}; + // Texturing state per unit, in increasing priority: bool m_texture_1d_enabled { false }; bool m_texture_2d_enabled { false };