Fix upload glyph image data to FontAtlas out of range (#2898)

halx99 · web-flow · commit 471031185e9f · 2025-11-28T02:30:02.000+08:00
diff --git a/axmol/2d/FontAtlas.cpp b/axmol/2d/FontAtlas.cpp
@@ -37,7 +37,6 @@
 #include "simdjson/simdjson.h"
 #include "zlib.h"
 #include "axmol/base/ZipUtils.h"
-
 #include "axmol/base/json.h"
 
 namespace ax
@@ -121,7 +120,9 @@ void FontAtlas::loadFontAtlas(std::string_view fontatlasFile, axstd::string_map<
 
 FontAtlas::FontAtlas(Font* theFont)
     : FontAtlas(theFont, CacheTextureWidth, CacheTextureHeight, AX_CONTENT_SCALE_FACTOR())
-{}
+{
+    _newChars.reserve(128);
+}
 
 FontAtlas::FontAtlas(Font* theFont, int atlasWidth, int atlasHeight, float scaleFactor)
     : _font(theFont), _width(atlasWidth), _height(atlasHeight), _scaleFactor(scaleFactor)
@@ -312,58 +313,57 @@ bool FontAtlas::getLetterDefinitionForChar(char32_t utf32Char, FontLetterDefinit
     }
 }
 
-void FontAtlas::findNewCharacters(const std::u32string& u32Text, std::unordered_set<char32_t>& charset)
+bool FontAtlas::findNewCharacters(const std::u32string& u32Text)
 {
+    _newChars.clear();
+
     if (_letterDefinitions.empty())
     {
-        std::copy(u32Text.begin(), u32Text.end(), std::inserter(charset, charset.end()));
+        std::copy(u32Text.begin(), u32Text.end(), std::inserter(_newChars, _newChars.end()));
     }
     else
     {
         for (auto&& charCode : u32Text)
             if (_letterDefinitions.find(charCode) == _letterDefinitions.end())
-                charset.insert(charCode);
+                _newChars.insert(charCode);
     }
+
+    return !_newChars.empty();
 }
 
 bool FontAtlas::prepareLetterDefinitions(const std::u32string& utf32Text)
 {
-    if (_fontFreeType == nullptr)
-    {
+    if (!_fontFreeType)
         return false;
-    }
-
     if (!_currentPageData)
         reinit();
 
-    std::unordered_set<char32_t> charCodeSet;
-    findNewCharacters(utf32Text, charCodeSet);
-    if (charCodeSet.empty())
-    {
+    if (!findNewCharacters(utf32Text))
         return false;
-    }
 
-    int adjustForDistanceMap = _letterPadding / 2;
-    int adjustForExtend      = _letterEdgeExtend / 2;
-    int bitmapWidth          = 0;
-    int bitmapHeight         = 0;
-    int glyphHeight;
-    Rect tempRect;
-    FontLetterDefinition tempDef;
+    const int adjustForDistanceMap = _letterPadding / 2;
+    const int adjustForExtend      = _letterEdgeExtend / 2;
 
-    auto pixelFormat = _pixelFormat;
+    int bitmapWidth = 0, bitmapHeight = 0;
+    Rect tempRect;
+    FontLetterDefinition tempDef{};
 
-    int startY = (int)_currentPageOrigY;
+    auto pixelFormat     = _pixelFormat;
+    int startY           = static_cast<int>(_currentPageOrigY);
+    int pageUploadStartY = startY;  // Upload start position of the current page
+    int pageUploadEndY   = startY;  // Upload end position of the current page (updated dynamically)
 
-    for (auto&& charCode : charCodeSet)
+    for (auto&& charCode : _newChars)
     {
         auto missingIt             = _missingGlyphFallbackFonts.find(charCode);
         uint8_t* bitmap            = nullptr;
         FontFreeType* charRenderer = _fontFreeType;
+        bool sharedBitmapData{true};  // does the bitmap data shared from FontFreeType engine
         if (missingIt == _missingGlyphFallbackFonts.end())
         {
             const GlyphResolution* res{nullptr};
-            bitmap = charRenderer->getGlyphBitmap(charCode, bitmapWidth, bitmapHeight, tempRect, tempDef.xAdvance, res);
+            bitmap = charRenderer->getGlyphBitmap(charCode, bitmapWidth, bitmapHeight, tempRect, tempDef.xAdvance, res,
+                                                  sharedBitmapData);
             if (!bitmap && res)
             {
                 auto fallbackIt = _missingFallbackFonts.find(res->faceInfo.family);
@@ -382,86 +382,122 @@ bool FontAtlas::prepareLetterDefinitions(const std::u32string& utf32Text)
                 {
                     unsigned int glyphIndex = res->glyphIndex;
                     bitmap = charRenderer->getGlyphBitmapByIndex(glyphIndex, bitmapWidth, bitmapHeight, tempRect,
-                                                                 tempDef.xAdvance);
+                                                                 tempDef.xAdvance, sharedBitmapData);
                     _missingGlyphFallbackFonts.emplace(charCode, std::make_pair(charRenderer, glyphIndex));
                 }
             }
         }
         else
-        {  // found fallback font for missing charas, getGlyphBitmap without fallback
+        {  // Found fallback font for missing characters, getGlyphBitmap without fallback
             charRenderer            = missingIt->second.first;
             unsigned int glyphIndex = missingIt->second.second;
-            bitmap =
-                charRenderer->getGlyphBitmapByIndex(glyphIndex, bitmapWidth, bitmapHeight, tempRect, tempDef.xAdvance);
+            bitmap = charRenderer->getGlyphBitmapByIndex(glyphIndex, bitmapWidth, bitmapHeight, tempRect,
+                                                         tempDef.xAdvance, sharedBitmapData);
         }
+
         if (bitmap && bitmapWidth > 0 && bitmapHeight > 0)
         {
-            tempDef.validDefinition = true;
-            tempDef.width           = tempRect.size.width + _letterPadding + _letterEdgeExtend;
-            tempDef.height          = tempRect.size.height + _letterPadding + _letterEdgeExtend;
-            tempDef.offsetX         = tempRect.origin.x - adjustForDistanceMap - adjustForExtend;
-            tempDef.offsetY         = _fontAscender + tempRect.origin.y - adjustForDistanceMap - adjustForExtend;
+            // Calculate occupied area using actual bitmap size
+            const int glyphWidth  = bitmapWidth + _letterPadding + _letterEdgeExtend;
+            const int glyphHeight = bitmapHeight + _letterPadding + _letterEdgeExtend;
 
-            if (_currentPageOrigX + tempDef.width > _width)
+            // Not enough width in current line → wrap to next line
+            if (_currentPageOrigX + glyphWidth > _width)
             {
                 _currentPageOrigY += _currLineHeight;
                 _currLineHeight   = 0;
                 _currentPageOrigX = 0;
-                if (_currentPageOrigY + _lineHeight + _letterPadding + _letterEdgeExtend >= _height)
-                {
-                    updateTextureContent(pixelFormat, startY);
-
-                    startY = 0;
+            }
 
-                    addNewPage();
-                }
+            // Not enough height → upload current page and start a new page
+            if (_currentPageOrigY + glyphHeight > _height)
+            {
+                // Upload the written region of the current page (startY..pageUploadEndY)
+                updateTextureContent(pixelFormat, pageUploadStartY);
+                // Start a new page
+                addNewPage();
+                _currentPageOrigX = 0;
+                _currentPageOrigY = 0;
+                _currLineHeight   = 0;
+                pageUploadStartY  = 0;
+                pageUploadEndY    = 0;
             }
-            glyphHeight     = static_cast<int>(bitmapHeight) + _letterPadding + _letterEdgeExtend;
-            _currLineHeight = std::max(glyphHeight, _currLineHeight);
-            charRenderer->renderCharAt(_currentPageData, (int)_currentPageOrigX + adjustForExtend,
-                                       (int)_currentPageOrigY + adjustForExtend, bitmap, bitmapWidth, bitmapHeight,
-                                       _width, _height);
 
+            // Calculate tempDef offsets (based on tempRect)
+            tempDef.validDefinition = true;
+            tempDef.width           = tempRect.size.width + _letterPadding + _letterEdgeExtend;
+            tempDef.height          = tempRect.size.height + _letterPadding + _letterEdgeExtend;
+            tempDef.offsetX         = tempRect.origin.x - adjustForDistanceMap - adjustForExtend;
+            tempDef.offsetY         = _fontAscender + tempRect.origin.y - adjustForDistanceMap - adjustForExtend;
+
+            // Render glyph into the current page
+            charRenderer->renderCharAt(_currentPageData, static_cast<int>(_currentPageOrigX) + adjustForExtend,
+                                       static_cast<int>(_currentPageOrigY) + adjustForExtend, bitmap, bitmapWidth,
+                                       bitmapHeight, _width, _height);
+
+            // Record glyph position and page
             tempDef.U         = _currentPageOrigX;
             tempDef.V         = _currentPageOrigY;
             tempDef.textureID = _currentPage;
-            _currentPageOrigX += tempDef.width + 1;
-            // take from pixels to points
-            tempDef.width   = tempDef.width / _scaleFactor;
-            tempDef.height  = tempDef.height / _scaleFactor;
-            tempDef.U       = tempDef.U / _scaleFactor;
-            tempDef.V       = tempDef.V / _scaleFactor;
+
+            // Update line height and X pointer (leave 1 pixel spacing between glyphs)
+            _currLineHeight = std::max(glyphHeight, _currLineHeight);
+            _currentPageOrigX += glyphWidth + 1;
+
+            // Maintain upload range (highest Y written on this page)
+            pageUploadEndY = std::max<int>(pageUploadEndY, _currentPageOrigY + _currLineHeight);
+
+            // Convert pixel dimensions to points
+            tempDef.width /= _scaleFactor;
+            tempDef.height /= _scaleFactor;
+            tempDef.U /= _scaleFactor;
+            tempDef.V /= _scaleFactor;
             tempDef.rotated = false;
         }
         else
         {
-            delete[] bitmap;
-
             tempDef.validDefinition = !!tempDef.xAdvance;
-            tempDef.width           = 0;
-            tempDef.height          = 0;
-            tempDef.U               = 0;
-            tempDef.V               = 0;
-            tempDef.offsetX         = 0;
-            tempDef.offsetY         = 0;
-            tempDef.textureID       = 0;
-            tempDef.rotated         = false;
+            tempDef.width = tempDef.height = tempDef.U = tempDef.V = 0;
+            tempDef.offsetX = tempDef.offsetY = 0;
+            tempDef.textureID                 = 0;
+            tempDef.rotated                   = false;
             _currentPageOrigX += 1;
         }
 
+        if (!sharedBitmapData && bitmap)
+            delete[] bitmap;
+
         _letterDefinitions[charCode] = tempDef;
     }
 
-    updateTextureContent(pixelFormat, startY);
+    // Handle remaining upload for the current page (from startY to max written Y)
+    if (pageUploadEndY > pageUploadStartY)
+    {
+        updateTextureContent(pixelFormat, pageUploadStartY);
+    }
 
     return true;
 }
 
 void FontAtlas::updateTextureContent(rhi::PixelFormat format, int startY)
 {
-    auto data = _currentPageData + (_width * (int)startY << _strideShift);
-    _atlasTextures[_currentPage]->updateSubData(data, 0, startY, _width,
-                                                (std::min)((int)_currentPageOrigY - startY + _currLineHeight, _height));
+    // Calculate the starting data pointer
+    auto data = _currentPageData + ((_width * (int)startY) << _strideShift);
+
+    // Calculate the actual upload height: from startY to _currentPageOrigY + _currLineHeight
+    int uploadHeight = (int)_currentPageOrigY + _currLineHeight - startY;
+
+    // Defensive check to avoid negative values or overflow
+    if (uploadHeight < 0)
+        uploadHeight = 0;
+
+    // Clamp to remaining space to prevent out-of-bounds
+    uploadHeight = std::min(uploadHeight, _height - startY);
+    if (uploadHeight <= 0)
+        return;
+
+    // Perform the texture update
+    _atlasTextures[_currentPage]->updateSubData(data, 0, startY, _width, uploadHeight);
 }
 
 void FontAtlas::addNewPage()
diff --git a/axmol/2d/FontAtlas.h b/axmol/2d/FontAtlas.h
@@ -40,6 +40,8 @@
 #include "axmol/base/Map.h"
 #include "axmol/2d/FontFreeType.h"
 
+#include "axmol/tlx/flat_set.hpp"
+
 namespace ax
 {
 
@@ -132,7 +134,7 @@ class AX_DLL FontAtlas : public Object
 
     void releaseTextures();
 
-    void findNewCharacters(const std::u32string& u32Text, std::unordered_set<char32_t>& charCodeSet);
+    bool findNewCharacters(const std::u32string& u32Text);
 
     /**
      * Scale each font letter by scaleFactor.
@@ -143,6 +145,8 @@ class AX_DLL FontAtlas : public Object
 
     void updateTextureContent(rhi::PixelFormat format, int startY);
 
+    axstd::flat_set<char32_t> _newChars;
+
     std::unordered_map<unsigned int, Texture2D*> _atlasTextures;
     std::unordered_map<char32_t, FontLetterDefinition> _letterDefinitions;
 
diff --git a/axmol/2d/FontFreeType.cpp b/axmol/2d/FontFreeType.cpp
@@ -424,7 +424,8 @@ unsigned char* FontFreeType::getGlyphBitmap(char32_t charCode,
                                             int& outHeight,
                                             Rect& outRect,
                                             int& xAdvance,
-                                            const GlyphResolution*& outFallbackRes)
+                                            const GlyphResolution*& outFallbackRes,
+                                            bool& sharedBitmapData)
 {
     unsigned char* ret = nullptr;
 
@@ -466,19 +467,21 @@ unsigned char* FontFreeType::getGlyphBitmap(char32_t charCode,
         }
     }
 
-    return getGlyphBitmapByIndex(glyphIndex, outWidth, outHeight, outRect, xAdvance);
+    return getGlyphBitmapByIndex(glyphIndex, outWidth, outHeight, outRect, xAdvance, sharedBitmapData);
 }
 
 unsigned char* FontFreeType::getGlyphBitmapByIndex(unsigned int glyphIndex,
                                                    int& outWidth,
                                                    int& outHeight,
                                                    Rect& outRect,
-                                                   int& xAdvance)
+                                                   int& xAdvance,
+                                                   bool& sharedBitmapData)
 {
     unsigned char* ret = nullptr;
 
     do
     {
+        sharedBitmapData = true;
         if (FT_Load_Glyph(_fontFace, glyphIndex, FT_LOAD_RENDER | FT_LOAD_NO_AUTOHINT))
             break;
 
@@ -503,6 +506,8 @@ unsigned char* FontFreeType::getGlyphBitmapByIndex(unsigned int glyphIndex,
 
         if (_outlineSize > 0 && outWidth > 0 && outHeight > 0)
         {
+            sharedBitmapData = false;
+
             auto copyBitmap = new unsigned char[outWidth * outHeight];
             memcpy(copyBitmap, ret, outWidth * outHeight * sizeof(unsigned char));
 
@@ -653,7 +658,6 @@ void FontFreeType::renderCharAt(unsigned char* dest,
             memcpy(dest + (iX + (iY * atlasWidth)) * 2, bitmap + bitmap_y * 2, bitmapWidth * 2);
             ++iY;
         }
-        delete[] bitmap;
     }
     else
     {
diff --git a/axmol/2d/FontFreeType.h b/axmol/2d/FontFreeType.h
@@ -129,13 +129,15 @@ class AX_DLL FontFreeType : public Font
                                   int& outHeight,
                                   Rect& outRect,
                                   int& xAdvance,
-                                  const GlyphResolution*& fallbackRes);
+                                  const GlyphResolution*& fallbackRes,
+                                  bool& sharedBitmapData);
 
     unsigned char* getGlyphBitmapByIndex(unsigned int glyphIndex,
                                          int& outWidth,
                                          int& outHeight,
                                          Rect& outRect,
-                                         int& xAdvance);
+                                         int& xAdvance,
+                                         bool& sharedBitmapData);
 
     int getFontAscender() const;
     const char* getFontFamily() const;
diff --git a/axmol/rhi/d3d12/Texture12.cpp b/axmol/rhi/d3d12/Texture12.cpp
@@ -142,6 +142,12 @@ void TextureImpl::updateSubData(int xoffset,
 
     // Query footprint for the subresource
     const auto texDesc = _nativeTexture.resource->GetDesc();
+
+    assert(xoffset >= 0 && yoffset >= 0);
+    assert(width > 0 && height > 0);
+    assert(xoffset + width <= static_cast<int>(texDesc.Width));
+    assert(yoffset + height <= static_cast<int>(texDesc.Height));
+
     D3D12_PLACED_SUBRESOURCE_FOOTPRINT footprint;
     UINT numRows;
     UINT64 rowSizeInBytes;
diff --git a/axmol/tlx/sorted_vector.hpp b/axmol/tlx/sorted_vector.hpp
@@ -165,6 +165,12 @@ class sorted_vector
         return {lower_bound(key), upper_bound(key)};
     }
 
+    iterator insert(iterator /*hint*/, const value_type& v)
+    {
+        // ignore hint, do sorted insertion
+        return insert(v).first;
+    }
+
     std::pair<iterator, bool> insert(const value_type& v)
     {
         auto& cont        = _Mypair.second();

Original file line number	Diff line number	Diff line change
`@@ -424,7 +424,8 @@ unsigned char* FontFreeType::getGlyphBitmap(char32_t charCode,`
`424`	`424`	`int& outHeight,`
`425`	`425`	`Rect& outRect,`
`426`	`426`	`int& xAdvance,`
`427`		`- const GlyphResolution*& outFallbackRes)`
	`427`	`+ const GlyphResolution*& outFallbackRes,`
	`428`	`+ bool& sharedBitmapData)`
`428`	`429`	`{`
`429`	`430`	`unsigned char* ret = nullptr;`
`430`	`431`
`@@ -466,19 +467,21 @@ unsigned char* FontFreeType::getGlyphBitmap(char32_t charCode,`
`466`	`467`	`}`
`467`	`468`	`}`
`468`	`469`
`469`		`- return getGlyphBitmapByIndex(glyphIndex, outWidth, outHeight, outRect, xAdvance);`
	`470`	`+ return getGlyphBitmapByIndex(glyphIndex, outWidth, outHeight, outRect, xAdvance, sharedBitmapData);`
`470`	`471`	`}`
`471`	`472`
`472`	`473`	`unsigned char* FontFreeType::getGlyphBitmapByIndex(unsigned int glyphIndex,`
`473`	`474`	`int& outWidth,`
`474`	`475`	`int& outHeight,`
`475`	`476`	`Rect& outRect,`
`476`		`- int& xAdvance)`
	`477`	`+ int& xAdvance,`
	`478`	`+ bool& sharedBitmapData)`
`477`	`479`	`{`
`478`	`480`	`unsigned char* ret = nullptr;`
`479`	`481`
`480`	`482`	`do`
`481`	`483`	`{`
	`484`	`+ sharedBitmapData = true;`
`482`	`485`	`if (FT_Load_Glyph(_fontFace, glyphIndex, FT_LOAD_RENDER \| FT_LOAD_NO_AUTOHINT))`
`483`	`486`	`break;`
`484`	`487`
`@@ -503,6 +506,8 @@ unsigned char* FontFreeType::getGlyphBitmapByIndex(unsigned int glyphIndex,`
`503`	`506`
`504`	`507`	`if (_outlineSize > 0 && outWidth > 0 && outHeight > 0)`
`505`	`508`	`{`
	`509`	`+ sharedBitmapData = false;`
	`510`	`+`
`506`	`511`	`auto copyBitmap = new unsigned char[outWidth * outHeight];`
`507`	`512`	`memcpy(copyBitmap, ret, outWidth * outHeight * sizeof(unsigned char));`
`508`	`513`
`@@ -653,7 +658,6 @@ void FontFreeType::renderCharAt(unsigned char* dest,`
`653`	`658`	`memcpy(dest + (iX + (iY * atlasWidth)) * 2, bitmap + bitmap_y * 2, bitmapWidth * 2);`
`654`	`659`	`++iY;`
`655`	`660`	`}`
`656`		`- delete[] bitmap;`
`657`	`661`	`}`
`658`	`662`	`else`
`659`	`663`	`{`
Original file line number	Diff line number	Diff line change
`@@ -165,6 +165,12 @@ class sorted_vector`
`165`	`165`	`return {lower_bound(key), upper_bound(key)};`
`166`	`166`	`}`
`167`	`167`
	`168`	`+ iterator insert(iterator /hint/, const value_type& v)`
	`169`	`+ {`
	`170`	`+ // ignore hint, do sorted insertion`
	`171`	`+ return insert(v).first;`
	`172`	`+ }`
	`173`	`+`
`168`	`174`	`std::pair<iterator, bool> insert(const value_type& v)`
`169`	`175`	`{`
`170`	`176`	`auto& cont = _Mypair.second();`