View Issue Details

IDProjectCategoryView StatusLast Update
0001753NoesisGUIC++ SDKpublic2020-07-13 13:25
ReporterstevehAssigned Tojsantos 
PrioritynormalSeveritymajorReproducibilityrandom
Status resolvedResolutionfixed 
Product Version3.0 
Target Version3.0.3Fixed in Version3.0.3 
Summary0001753: Assert in GlyphCache::GetGlyph
DescriptionHi guys, we're seeing a rare assert only on Xbox and fairly infrequent. It seems to be going down in GlyphCache::GetGlyph when inserting a new glyph into the hash set:

        return (Glyph*)&*mCache.Insert(MoveArg(glyph)).first;

I don't have a dump to go on, I only have logs / callstack. I'm in the process of adding some additional logging to try and trap this. So the assert ultimately comes from the hash map insert:

HashMapImpl<Bucket>::InsertIntoBucket


It's going down on the last assert:

    NS_ASSERT(!bucket->IsEmpty());


So, it has a valid bucket, it's tried to insert the entry, but it still thinks the bucket is empty. It looks like the GlyphCache uses a custom key info struct:



    struct GlyphKeyInfo
    {
        static bool IsEmpty(const Glyph& k) { return k.x == 0xffff; }
        static void MarkEmpty(Glyph& k) { k.x = 0xffff; }
    // ...
     }

So it thinks the bucket is empty if the "x" property of the Glyph element is 0xffff. So I went back to the function and there's a few things I noticed.


First, there's some macros in the CPP at the top:


#define MARK_GLYPH_EMPTY(g) ((g)->width = 0xffff)
#define IS_GLYPH_EMPTY(g) ((g)->width == 0xffff)

#define MARK_GLYPH_UNLOADED(g) ((g)->width = 0xffff - 1)
#define IS_GLYPH_UNLOADED(g) ((g)->width == 0xffff - 1)


These macros use the width property. Neither of these affect the x property at all. Then going back to the original function GetGlyph:


On line 497 it constructs a new glyph on the stack, at this point all properties are uninitialised. If it's a free type font, I don't see where the x / y properties of the glyph will be initialised, it looks like it only ever initialises the layers array in the glyph, would this just not randomly assert depending on the memory of the x / y properties which is undefined behaviour?

In the GlyphCache::RenderSDF, it looks like it will only ever initialise the x / y values of the glyph if there are some contours of the outline, otherwise it'll call MARK_GLYPH_EMPTY(..) which will initialise the width, but not the x parameter. Again, isn't this undefined behaviour?

This potential undefined behaviour certainly seems to line up with what we're seeing with fairly rare random asserts. Is the GlyphKeyInfo supposed to read from the width property instead of the x property?
Steps To Reproduce1. Render text
2. Rarely, the game will assert when trying to build the glyph cache for the font
TagsNo tags attached.
PlatformXboxOne

Activities

jsantos

jsantos

2020-07-09 18:11

manager   ~0006528

Hi Steve, thanks for reporting this. Another client already reported a similar thing and I am working on it. For now, I don't need more information or dumps because I think I understand what's happening.

Will provide a fix ASAP.
jsantos

jsantos

2020-07-09 18:12

manager   ~0006529

And, as always, thanks for the detailed report. There are many useful things there that will help me.
jsantos

jsantos

2020-07-10 12:18

manager   ~0006530

Last edited: 2020-07-10 18:41

View 3 revisions

Hi Steve, effectively there is a bug in the code, in some cases the x property is not properly initialized and we are using the special value 0xffff to mark empty buckets in the hashmap (unfortunately we are using the term empty here to mean two things, an empty bucket and a glyph without outlines, we should improve this to avoid confusion but for now I am not changing it).

Could you please try the following patch?

Index: GlyphCache.cpp
===================================================================
--- GlyphCache.cpp	(revision 9396)
+++ GlyphCache.cpp	(working copy)
@@ -32,10 +32,10 @@
 
 #define TILE_SIZE 512
 
-#define MARK_GLYPH_EMPTY(g) ((g)->width = 0xffff)
+#define MARK_GLYPH_EMPTY(g) (g)->x = 0; (g)->width = 0xffff
 #define IS_GLYPH_EMPTY(g) ((g)->width == 0xffff)
 
-#define MARK_GLYPH_UNLOADED(g) ((g)->width = 0xffff - 1)
+#define MARK_GLYPH_UNLOADED(g) (g)->x = 0; (g)->width = 0xffff - 1
 #define IS_GLYPH_UNLOADED(g) ((g)->width == 0xffff - 1)
 
 #ifdef NS_PROFILE


Issue History

Date Modified Username Field Change
2020-07-09 17:33 steveh New Issue
2020-07-09 18:11 jsantos Note Added: 0006528
2020-07-09 18:11 jsantos Assigned To => jsantos
2020-07-09 18:11 jsantos Status new => assigned
2020-07-09 18:11 jsantos Target Version => 3.0.3
2020-07-09 18:12 jsantos Note Added: 0006529
2020-07-10 12:18 jsantos Note Added: 0006530
2020-07-10 12:18 jsantos Note Edited: 0006530 View Revisions
2020-07-10 12:47 jsantos Status assigned => feedback
2020-07-10 18:41 jsantos Note Edited: 0006530 View Revisions
2020-07-13 13:25 jsantos Status feedback => resolved
2020-07-13 13:25 jsantos Resolution open => fixed
2020-07-13 13:25 jsantos Fixed in Version => 3.0.3