View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001753 | NoesisGUI | C++ SDK | public | 2020-07-09 17:33 | 2020-07-13 13:25 |
Reporter | steveh | Assigned To | jsantos | ||
Priority | normal | Severity | major | Reproducibility | random |
Status | resolved | Resolution | fixed | ||
Product Version | 3.0 | ||||
Target Version | 3.0.3 | Fixed in Version | 3.0.3 | ||
Summary | 0001753: Assert in GlyphCache::GetGlyph | ||||
Description | Hi 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 Reproduce | 1. Render text 2. Rarely, the game will assert when trying to build the glyph cache for the font | ||||
Tags | No tags attached. | ||||
Platform | XboxOne | ||||
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. |
|
And, as always, thanks for the detailed report. There are many useful things there that will help me. | |
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 |
|
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 | |
2020-07-10 12:47 | jsantos | Status | assigned => feedback |
2020-07-10 18:41 | jsantos | Note Edited: 0006530 | |
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 |