From eecaf4cdfc949a46d7d94c140e8e52e124763280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 31 Oct 2024 17:55:01 +0100 Subject: [PATCH 1/4] Remove redundant `BmpWidth`/`BmpHeight` variables They are identical to the `Width`/`Height` variables which specify the size of the glyph in the atlas texture. --- src/engine/client/text.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/engine/client/text.cpp b/src/engine/client/text.cpp index 86eb6f891..dc2646e2e 100644 --- a/src/engine/client/text.cpp +++ b/src/engine/client/text.cpp @@ -558,9 +558,6 @@ private: // set glyph info { - const int BmpWidth = pBitmap->width + x * 2; - const int BmpHeight = pBitmap->rows + y * 2; - Glyph.m_Height = Height; Glyph.m_Width = Width; Glyph.m_CharHeight = RealHeight; @@ -571,8 +568,8 @@ private: Glyph.m_aUVs[0] = X; Glyph.m_aUVs[1] = Y; - Glyph.m_aUVs[2] = Glyph.m_aUVs[0] + BmpWidth; - Glyph.m_aUVs[3] = Glyph.m_aUVs[1] + BmpHeight; + Glyph.m_aUVs[2] = Glyph.m_aUVs[0] + Width; + Glyph.m_aUVs[3] = Glyph.m_aUVs[1] + Height; Glyph.m_State = SGlyph::EState::RENDERED; } From f5179df6d02e6a3df78d22e5ced85bb5da714223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 31 Oct 2024 23:50:39 +0100 Subject: [PATCH 2/4] Avoid unnecessary copy of glyph data for entities text Copy the glyph data directly from the font bitmap buffer to the target image instead of using `m_aaGlyphData` as a temporary buffer. --- src/engine/client/text.cpp | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/engine/client/text.cpp b/src/engine/client/text.cpp index dc2646e2e..cd56be482 100644 --- a/src/engine/client/text.cpp +++ b/src/engine/client/text.cpp @@ -788,14 +788,6 @@ public: } const FT_Bitmap *pBitmap = &Face->glyph->bitmap; - - // prepare glyph data - const size_t GlyphDataSize = (size_t)pBitmap->width * pBitmap->rows * sizeof(uint8_t); - if(pBitmap->pixel_mode == FT_PIXEL_MODE_GRAY) - mem_copy(m_aaGlyphData[FONT_TEXTURE_FILL], pBitmap->buffer, GlyphDataSize); - else - mem_zero(m_aaGlyphData[FONT_TEXTURE_FILL], GlyphDataSize); - for(unsigned OffY = 0; OffY < pBitmap->rows; ++OffY) { for(unsigned OffX = 0; OffX < pBitmap->width; ++OffX) @@ -803,18 +795,11 @@ public: const int ImgOffX = clamp(x + OffX + WidthLastChars, x, (x + TexSubWidth) - 1); const int ImgOffY = clamp(y + OffY, y, (y + TexSubHeight) - 1); const size_t ImageOffset = ImgOffY * (TextImage.m_Width * PixelSize) + ImgOffX * PixelSize; - const size_t GlyphOffset = OffY * pBitmap->width + OffX; - for(size_t i = 0; i < PixelSize; ++i) + for(size_t i = 0; i < PixelSize - 1; ++i) { - if(i != PixelSize - 1) - { - *(TextImage.m_pData + ImageOffset + i) = 255; - } - else - { - *(TextImage.m_pData + ImageOffset + i) = *(m_aaGlyphData[FONT_TEXTURE_FILL] + GlyphOffset); - } + TextImage.m_pData[ImageOffset + i] = 255; } + TextImage.m_pData[ImageOffset + PixelSize - 1] = pBitmap->buffer[OffY * pBitmap->width + OffX]; } } From a7a5c0ea7a3b4c4500f6af6fa24a08c530a61033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 31 Oct 2024 23:54:12 +0100 Subject: [PATCH 3/4] Add checks for unsupported glyph pixel mode Skip rendering glyphs which don't use the 1-byte grayscale pixel mode. --- src/engine/client/text.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/engine/client/text.cpp b/src/engine/client/text.cpp index cd56be482..93aeeb54b 100644 --- a/src/engine/client/text.cpp +++ b/src/engine/client/text.cpp @@ -510,6 +510,11 @@ private: } const FT_Bitmap *pBitmap = &Glyph.m_Face->glyph->bitmap; + if(pBitmap->pixel_mode != FT_PIXEL_MODE_GRAY) + { + log_debug("textrender", "Error loading glyph, unsupported pixel mode. Chr=%d GlyphIndex=%u PixelMode=%d", Glyph.m_Chr, Glyph.m_GlyphIndex, pBitmap->pixel_mode); + return false; + } const unsigned RealWidth = pBitmap->width; const unsigned RealHeight = pBitmap->rows; @@ -788,6 +793,13 @@ public: } const FT_Bitmap *pBitmap = &Face->glyph->bitmap; + if(pBitmap->pixel_mode != FT_PIXEL_MODE_GRAY) + { + log_debug("textrender", "Error loading glyph, unsupported pixel mode. Chr=%d GlyphIndex=%u PixelMode=%d", NextCharacter, GlyphIndex, pBitmap->pixel_mode); + pCurrent = pTmp; + continue; + } + for(unsigned OffY = 0; OffY < pBitmap->rows; ++OffY) { for(unsigned OffX = 0; OffX < pBitmap->width; ++OffX) From fe78331e803f9859fe1d2105853d663f69df1016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Fri, 1 Nov 2024 00:02:28 +0100 Subject: [PATCH 4/4] More efficient glyph uploading, fix crash with very large glyphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid using the `m_aaGlyphData` temporary buffer for uploading glyphs. Instead, allocate the required memory for the glyphs directly and allow the graphics backend to take ownership of the buffer when updating text textures. This avoids copying the glyph data into the temporary buffer when uploading individual glyphs. This also avoids crashes when rendering very large glyphs with large font sizes, due to the buffer `m_aaGlyphData` having a fixed size of `64 * 1024` while the maximum glyph size is not checked. This fixed size could be exceeded with glyphs larger than 256² in rendered dimensions. There should currently be no glyph in our fonts which is that large and also no font size so large that this could happen. --- src/engine/client/graphics_threaded.cpp | 17 ++++++++++++----- src/engine/client/graphics_threaded.h | 2 +- src/engine/client/text.cpp | 22 +++++++++++----------- src/engine/graphics.h | 2 +- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/engine/client/graphics_threaded.cpp b/src/engine/client/graphics_threaded.cpp index 8dc82c34a..a9bf3d4a5 100644 --- a/src/engine/client/graphics_threaded.cpp +++ b/src/engine/client/graphics_threaded.cpp @@ -484,7 +484,7 @@ bool CGraphics_Threaded::UnloadTextTextures(CTextureHandle &TextTexture, CTextur return true; } -bool CGraphics_Threaded::UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, const uint8_t *pData) +bool CGraphics_Threaded::UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, uint8_t *pData, bool IsMovedPointer) { CCommandBuffer::SCommand_TextTexture_Update Cmd; Cmd.m_Slot = TextureId.Id(); @@ -493,10 +493,17 @@ bool CGraphics_Threaded::UpdateTextTexture(CTextureHandle TextureId, int x, int Cmd.m_Width = Width; Cmd.m_Height = Height; - const size_t MemSize = Width * Height; - uint8_t *pTmpData = static_cast(malloc(MemSize)); - mem_copy(pTmpData, pData, MemSize); - Cmd.m_pData = pTmpData; + if(IsMovedPointer) + { + Cmd.m_pData = pData; + } + else + { + const size_t MemSize = Width * Height; + uint8_t *pTmpData = static_cast(malloc(MemSize)); + mem_copy(pTmpData, pData, MemSize); + Cmd.m_pData = pTmpData; + } AddCmd(Cmd); return true; diff --git a/src/engine/client/graphics_threaded.h b/src/engine/client/graphics_threaded.h index 01774d58f..1a7b023eb 100644 --- a/src/engine/client/graphics_threaded.h +++ b/src/engine/client/graphics_threaded.h @@ -950,7 +950,7 @@ public: bool LoadTextTextures(size_t Width, size_t Height, CTextureHandle &TextTexture, CTextureHandle &TextOutlineTexture, uint8_t *pTextData, uint8_t *pTextOutlineData) override; bool UnloadTextTextures(CTextureHandle &TextTexture, CTextureHandle &TextOutlineTexture) override; - bool UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, const uint8_t *pData) override; + bool UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, uint8_t *pData, bool IsMovedPointer) override; CTextureHandle LoadSpriteTexture(const CImageInfo &FromImageInfo, const struct CDataSprite *pSprite) override; diff --git a/src/engine/client/text.cpp b/src/engine/client/text.cpp index 93aeeb54b..252f80f03 100644 --- a/src/engine/client/text.cpp +++ b/src/engine/client/text.cpp @@ -321,9 +321,6 @@ private: CAtlas m_TextureAtlas; std::unordered_map, SGlyph, SGlyphKeyHash, SGlyphKeyEquals> m_Glyphs; - // Data used for rendering glyphs - uint8_t m_aaGlyphData[NUM_FONT_TEXTURES][64 * 1024]; - // Font faces FT_Face m_DefaultFace = nullptr; FT_Face m_IconFace = nullptr; @@ -485,13 +482,13 @@ private: return OutlineThickness; } - void UploadGlyph(int TextureIndex, int PosX, int PosY, size_t Width, size_t Height, const unsigned char *pData) + void UploadGlyph(int TextureIndex, int PosX, int PosY, size_t Width, size_t Height, uint8_t *pData) { for(size_t y = 0; y < Height; ++y) { mem_copy(&m_apTextureData[TextureIndex][PosX + ((y + PosY) * m_TextureDimension)], &pData[y * Width], Width); } - Graphics()->UpdateTextTexture(m_aTextures[TextureIndex], PosX, PosY, Width, Height, pData); + Graphics()->UpdateTextTexture(m_aTextures[TextureIndex], PosX, PosY, Width, Height, pData, true); } bool FitGlyph(size_t Width, size_t Height, int &PosX, int &PosY) @@ -549,16 +546,19 @@ private: } // prepare glyph data - mem_zero(m_aaGlyphData[FONT_TEXTURE_FILL], (size_t)Width * Height * sizeof(uint8_t)); + const size_t GlyphDataSize = (size_t)Width * Height * sizeof(uint8_t); + uint8_t *pGlyphDataFill = static_cast(malloc(GlyphDataSize)); + uint8_t *pGlyphDataOutline = static_cast(malloc(GlyphDataSize)); + mem_zero(pGlyphDataFill, GlyphDataSize); for(unsigned py = 0; py < pBitmap->rows; ++py) { - mem_copy(&m_aaGlyphData[FONT_TEXTURE_FILL][(py + y) * Width + x], &pBitmap->buffer[py * pBitmap->width], pBitmap->width); + mem_copy(&pGlyphDataFill[(py + y) * Width + x], &pBitmap->buffer[py * pBitmap->width], pBitmap->width); } + Grow(pGlyphDataFill, pGlyphDataOutline, Width, Height, OutlineThickness); // upload the glyph - UploadGlyph(FONT_TEXTURE_FILL, X, Y, Width, Height, m_aaGlyphData[FONT_TEXTURE_FILL]); - Grow(m_aaGlyphData[FONT_TEXTURE_FILL], m_aaGlyphData[FONT_TEXTURE_OUTLINE], Width, Height, OutlineThickness); - UploadGlyph(FONT_TEXTURE_OUTLINE, X, Y, Width, Height, m_aaGlyphData[FONT_TEXTURE_OUTLINE]); + UploadGlyph(FONT_TEXTURE_FILL, X, Y, Width, Height, pGlyphDataFill); + UploadGlyph(FONT_TEXTURE_OUTLINE, X, Y, Width, Height, pGlyphDataOutline); } // set glyph info @@ -696,7 +696,7 @@ public: for(size_t TextureIndex = 0; TextureIndex < NUM_FONT_TEXTURES; ++TextureIndex) { mem_zero(m_apTextureData[TextureIndex], m_TextureDimension * m_TextureDimension * sizeof(uint8_t)); - Graphics()->UpdateTextTexture(m_aTextures[TextureIndex], 0, 0, m_TextureDimension, m_TextureDimension, m_apTextureData[TextureIndex]); + Graphics()->UpdateTextTexture(m_aTextures[TextureIndex], 0, 0, m_TextureDimension, m_TextureDimension, m_apTextureData[TextureIndex], false); } m_TextureAtlas.Clear(m_TextureDimension); diff --git a/src/engine/graphics.h b/src/engine/graphics.h index 3699c3ea7..610f46e30 100644 --- a/src/engine/graphics.h +++ b/src/engine/graphics.h @@ -284,7 +284,7 @@ public: // pTextData & pTextOutlineData are automatically free'd virtual bool LoadTextTextures(size_t Width, size_t Height, CTextureHandle &TextTexture, CTextureHandle &TextOutlineTexture, uint8_t *pTextData, uint8_t *pTextOutlineData) = 0; virtual bool UnloadTextTextures(CTextureHandle &TextTexture, CTextureHandle &TextOutlineTexture) = 0; - virtual bool UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, const uint8_t *pData) = 0; + virtual bool UpdateTextTexture(CTextureHandle TextureId, int x, int y, size_t Width, size_t Height, uint8_t *pData, bool IsMovedPointer) = 0; virtual CTextureHandle LoadSpriteTexture(const CImageInfo &FromImageInfo, const struct CDataSprite *pSprite) = 0;