From 02e598861a67b1f401648b3ddbcb44892f59e451 Mon Sep 17 00:00:00 2001 From: mwilsnd <53413200+mwilsnd@users.noreply.github.com> Date: Wed, 7 Jun 2023 12:52:16 -0500 Subject: [PATCH] Fix Texture2D upload unpack alignment, filtering (#1219) --- include/mbgl/gfx/texture2d.hpp | 14 ++++++----- include/mbgl/gl/texture2d.hpp | 1 + src/mbgl/gl/texture2d.cpp | 41 ++++++++++++++++++++------------- src/mbgl/tile/geometry_tile.cpp | 6 ++++- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/include/mbgl/gfx/texture2d.hpp b/include/mbgl/gfx/texture2d.hpp index 6e5c3d2e4..3e5539cf0 100644 --- a/include/mbgl/gfx/texture2d.hpp +++ b/include/mbgl/gfx/texture2d.hpp @@ -20,10 +20,8 @@ class TextureResource; class Texture2D { public: struct SamplerState { - /// Minification filter within a single mip level - TextureFilterType minification{TextureFilterType::Nearest}; - /// Magnification filter within a single mip level - TextureFilterType magnification{TextureFilterType::Nearest}; + /// Minification and magnification filter within a single mip level + TextureFilterType filter{TextureFilterType::Nearest}; /// Wrapping behavior along U coordinate TextureWrapType wrapU{TextureWrapType::Clamp}; /// Wrapping behavior along V coordinate @@ -78,11 +76,15 @@ public: /// @brief Upload image data to the texture resource /// @param pixelData Image data to transfer virtual void upload(const void* pixelData, const Size& size_) noexcept = 0; + + /// @brief Upload image data to the texture resource + /// @tparam Image Image object type + /// @param img Image to transfer template void upload(const Image& img) noexcept { setFormat(Image::channels == 1 ? gfx::TexturePixelType::Alpha : gfx::TexturePixelType::RGBA, gfx::TextureChannelDataType::UnsignedByte); - upload(&img.data[0], img.size); + upload(img.data ? img.data.get() : nullptr, img.size); } virtual void uploadSubRegion(const void* pixelData, @@ -95,7 +97,7 @@ public: assert(Image::channels == getPixelStride()); assert(img.size.width + xOffset <= getSize().width); assert(img.size.height + yOffset <= getSize().height); - uploadSubRegion(&img.data[0], img.size, xOffset, yOffset); + uploadSubRegion(img.data ? img.data.get() : nullptr, img.size, xOffset, yOffset); } /// @brief Upload staged image data if present and required. diff --git a/include/mbgl/gl/texture2d.hpp b/include/mbgl/gl/texture2d.hpp index eb94e3f1f..e9d7c96e6 100644 --- a/include/mbgl/gl/texture2d.hpp +++ b/include/mbgl/gl/texture2d.hpp @@ -78,6 +78,7 @@ private: std::shared_ptr image{nullptr}; Size size{0, 0}; bool samplerStateDirty{false}; + bool storageDirty{false}; int32_t boundTextureUnit{-1}; int32_t boundLocation{-1}; diff --git a/src/mbgl/gl/texture2d.cpp b/src/mbgl/gl/texture2d.cpp index 4099ffdad..322ae84d1 100644 --- a/src/mbgl/gl/texture2d.cpp +++ b/src/mbgl/gl/texture2d.cpp @@ -6,7 +6,6 @@ #include #include -#include namespace mbgl { namespace gl { @@ -30,11 +29,13 @@ Texture2D& Texture2D::setFormat(gfx::TexturePixelType pixelFormat_, gfx::Texture assert(!textureResource); pixelFormat = pixelFormat_; channelType = channelType_; + storageDirty = true; return *this; } Texture2D& Texture2D::setSize(mbgl::Size size_) noexcept { size = size_; + storageDirty = true; return *this; } @@ -89,7 +90,7 @@ void Texture2D::createStorage(const void* data) noexcept { // Bind to TU 0 and upload context.activeTextureUnit = 0; context.texture[0] = getTextureID(); - updateSamplerConfiguration(); + context.pixelStoreUnpack = {1}; MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D, 0, Enum::to(pixelFormat), @@ -99,6 +100,8 @@ void Texture2D::createStorage(const void* data) noexcept { Enum::to(pixelFormat), Enum::to(channelType), data)); + storageDirty = false; + updateSamplerConfiguration(); } void Texture2D::create() noexcept { @@ -114,20 +117,24 @@ void Texture2D::updateSamplerConfiguration() noexcept { using namespace platform; samplerStateDirty = false; - MBGL_CHECK_ERROR( - glTexParameteri(GL_TEXTURE_2D, - GL_TEXTURE_MIN_FILTER, - samplerState.minification == gfx::TextureFilterType::Nearest ? GL_NEAREST : GL_LINEAR)); - MBGL_CHECK_ERROR( - glTexParameteri(GL_TEXTURE_2D, - GL_TEXTURE_MAG_FILTER, - samplerState.magnification == gfx::TextureFilterType::Nearest ? GL_NEAREST : GL_LINEAR)); + MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, + GL_TEXTURE_MIN_FILTER, + samplerState.filter == gfx::TextureFilterType::Nearest ? GL_NEAREST : GL_LINEAR)); + MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, + GL_TEXTURE_MAG_FILTER, + samplerState.filter == gfx::TextureFilterType::Nearest ? GL_NEAREST : GL_LINEAR)); MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, samplerState.wrapU == gfx::TextureWrapType::Clamp ? GL_CLAMP_TO_EDGE : GL_REPEAT)); MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, samplerState.wrapV == gfx::TextureWrapType::Clamp ? GL_CLAMP_TO_EDGE : GL_REPEAT)); + + // Keep the resource sampler configuration in sync + auto& glResource = static_cast(*textureResource); + glResource.filter = samplerState.filter; + glResource.wrapX = samplerState.wrapU; + glResource.wrapY = samplerState.wrapV; } void Texture2D::bind(int32_t location, int32_t textureUnit) noexcept { @@ -170,20 +177,21 @@ void Texture2D::unbind() noexcept { } void Texture2D::upload(const void* pixelData, const Size& size_) noexcept { - using namespace platform; + if (!textureResource || storageDirty || size_ == Size{0, 0} || size_ != size) { + size = size_; - if (!textureResource || size_ == Size{0, 0} || size_ != size) { // Create the texture object if we don't already have one if (!textureResource) { createObject(); } - size = size_; createStorage(pixelData); } else { - // Upload to existing memory - uploadSubRegion(pixelData, size, 0, 0); + if (pixelData) { + // Upload to existing memory + uploadSubRegion(pixelData, size, 0, 0); + } } } @@ -196,6 +204,7 @@ void Texture2D::uploadSubRegion(const void* pixelData, const Size& size_, uint16 // Bind to TU 0 and upload context.activeTextureUnit = 0; context.texture[0] = getTextureID(); + context.pixelStoreUnpack = {1}; MBGL_CHECK_ERROR(glTexSubImage2D(GL_TEXTURE_2D, 0, xOffset, @@ -210,7 +219,7 @@ void Texture2D::uploadSubRegion(const void* pixelData, const Size& size_, uint16 void Texture2D::upload() noexcept { if (image && image->valid()) { setFormat(gfx::TexturePixelType::RGBA, gfx::TextureChannelDataType::UnsignedByte); - upload(&(*image).data[0], image->size); + upload(image->data.get(), image->size); image.reset(); } } diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 588bd4ade..535f2494b 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -86,8 +86,12 @@ void GeometryTileRenderData::upload(gfx::UploadPass& uploadPass) { assert(atlasTextures); - if (layoutResult->glyphAtlasImage) { + if (layoutResult->glyphAtlasImage && layoutResult->glyphAtlasImage->valid()) { atlasTextures->glyph = uploadPass.getContext().createTexture2D(); + atlasTextures->glyph->setSamplerConfiguration({ + gfx::TextureFilterType::Linear, + gfx::TextureWrapType::Clamp, + gfx::TextureWrapType::Clamp}); atlasTextures->glyph->upload(*layoutResult->glyphAtlasImage); layoutResult->glyphAtlasImage = {}; }