From 917690c933974879bb0a4ddf53a2693cb8244b25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Sun, 6 Jul 2025 14:44:49 +0200
Subject: [PATCH] Text: set pixel storage image height always, and reset only
 if not used.

This makes the test added in previous commit pass on Chrome. This also
fixes the UI library there now, before it didn't show icons exactly due
to this, because they're uploaded with a non-zero offset (after text
glyphs) and Chrome requires the image height to be set in that case even
if uploading to the very first slice.
---
 src/Magnum/Text/AbstractGlyphCache.cpp        | 19 +++++++++++--------
 src/Magnum/Text/DistanceFieldGlyphCacheGL.cpp | 16 ++++++++++++----
 src/Magnum/Text/GlyphCacheGL.cpp              | 12 ++++++++++--
 3 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/Magnum/Text/AbstractGlyphCache.cpp b/src/Magnum/Text/AbstractGlyphCache.cpp
index e7762ffe7..a76205420 100644
--- a/src/Magnum/Text/AbstractGlyphCache.cpp
+++ b/src/Magnum/Text/AbstractGlyphCache.cpp
@@ -366,16 +366,19 @@ void AbstractGlyphCache::flushImage(const Range3Di& range) {
     const Vector3i paddedMax = Math::min(size(),
         range.max() + Vector3i{padding(), 0});
 
+    /* In case a subclass doesn't support image slicing (such as GlyphCacheGL
+       on WebGL 1), it'll set the image as a whole without the storage
+       parameters. In case it doesn't support 3D images (such as GlyphCacheGL
+       on GLES2), it'll clear the row length. OTOH, image height needs to be
+       set even in case of single-layer array images to avoid errors on certain
+       platforms. See GlyphCacheGLTest::setImageArraySingleLayer() for a repro
+       case. */
     /** @todo ugh have slicing on images directly already */
-    PixelStorage storage;
-    storage.setRowLength(state.image.size().x())
-        .setSkip(paddedMin);
-    /* Set image height only if it's an array glyph cache, as otherwise it'd
-       cause errors on ES2 that doesn't support this pixel storage state */
-    if(state.image.size().z() != 1)
-        storage.setImageHeight(state.image.size().y());
     doSetImage(paddedMin, ImageView3D{
-        storage,
+        PixelStorage{}
+            .setRowLength(state.image.size().x())
+            .setImageHeight(state.image.size().y())
+            .setSkip(paddedMin),
         state.image.format(),
         paddedMax - paddedMin,
         state.image.data()});
diff --git a/src/Magnum/Text/DistanceFieldGlyphCacheGL.cpp b/src/Magnum/Text/DistanceFieldGlyphCacheGL.cpp
index 327c2451b..f395a312a 100644
--- a/src/Magnum/Text/DistanceFieldGlyphCacheGL.cpp
+++ b/src/Magnum/Text/DistanceFieldGlyphCacheGL.cpp
@@ -182,13 +182,21 @@ void DistanceFieldGlyphCacheGL::doSetImage(const Vector2i&
     #endif
     #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL))
     {
+        /* If EXT_unpack_subimage is supported, use the storage as-is but
+           reset image height to 0 as that only matters with arrays which are
+           not supported on ES2 at all. It's set by AbstractGlyphCache always
+           because with array textures the upload may fail if not set. See
+           DistanceFieldGlyphCacheGLTest::setImageArraySingleLayer() for a
+           repro case. */
+        PixelStorage storage{image.storage()};
+        storage.setImageHeight(0);
+
         /* The image range was already expanded to include the padding in
            flushImage() */
-        CORRADE_INTERNAL_ASSERT(image.storage().skip().xy() == offset);
-        const Range2Di paddedRange = paddedImageRange(size(), image.storage().skip().xy(), image.size(), ratio);
+        CORRADE_INTERNAL_ASSERT(storage.skip().xy() == offset);
+        const Range2Di paddedRange = paddedImageRange(size(), storage.skip().xy(), image.size(), ratio);
         const ImageView2D paddedImage{
-            PixelStorage{image.storage()}
-                .setSkip({paddedRange.min(), image.storage().skip().z()}),
+            storage.setSkip({paddedRange.min(), storage.skip().z()}),
             image.format(),
             paddedRange.size(),
             image.data()};
diff --git a/src/Magnum/Text/GlyphCacheGL.cpp b/src/Magnum/Text/GlyphCacheGL.cpp
index 61036f5d2..54750ba0e 100644
--- a/src/Magnum/Text/GlyphCacheGL.cpp
+++ b/src/Magnum/Text/GlyphCacheGL.cpp
@@ -157,15 +157,23 @@ void GlyphCacheGL::doSetImage(const Vector2i& offset, const ImageView2D& image)
     #endif
     #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL))
     {
+        /* If EXT_unpack_subimage is supported, use the storage as-is but
+           reset image height to 0 as that only matters with arrays which are
+           not supported on ES2 at all. It's set by AbstractGlyphCache always
+           because with array textures the upload may fail if not set. See
+           GlyphCacheGLTest::setImageArraySingleLayer() for a repro case. */
+        PixelStorage storage{image.storage()};
+        storage.setImageHeight(0);
+
         /* On ES2 if EXT_texture_rg is present, the single-channel texture
            format is Red instead of Luminance */
         #ifdef MAGNUM_TARGET_GLES2
         if(image.format() == PixelFormat::R8Unorm && GL::Context::current().isExtensionSupported<GL::Extensions::EXT::texture_rg>()) {
-            state.texture.setSubImage(0, offset, ImageView2D{image.storage(), GL::PixelFormat::Red, GL::PixelType::UnsignedByte, image.size(), image.data()});
+            state.texture.setSubImage(0, offset, ImageView2D{storage, GL::PixelFormat::Red, GL::PixelType::UnsignedByte, image.size(), image.data()});
         } else
         #endif
         {
-            state.texture.setSubImage(0, offset, image);
+            state.texture.setSubImage(0, offset, ImageView2D{storage, image.format(), image.size(), image.data()});
         }
     }
     #endif