0001753: Assert in GlyphCache::GetGlyph - NoesisGUI Issue Tracker

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

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 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 2020-07-10 12:18 manager ~0006530 Last edited: 2020-07-10 18:41	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