BTW: Why should the caches be thread_local anyway? Isn't it true that Painter will use the same fonts with a whole bunch of threads when painting in MT? Having them common for all threads would have the used fonts cached after first hit and available to all threads. (Well, I'm likely missing something important here, but you'll probably educate me on that shortly...)

Tom1 wrote on Wed, 16 January 2019 08:54

Good morning Mirek,

Removing "thread__" from "thread__ CharEntry fc_cache[512];" fixes it.

Removing "thread__" from "thread__ FontEntry fi_cache[63];" does not have any effect on the problem.

Best regards,

Tom

BTW: Why should the caches be thread_local anyway? Isn't it true that Painter will use the same fonts with a whole bunch of threads when painting in MT? Having them common for all threads would have the used fonts cached after first hit and available to all threads. (Well, I'm likely missing something important here, but you'll probably educate me on that shortly...)

Only to avoid confusion, how the code for CharEntry exactly looks now for you? And again, the rest of the code is absolutely the same as trunk?

Mirek

CharEntry fc_cache[512];

thread__ CharEntry fc_cache[512];

mirek wrote on Wed, 16 January 2019 10:18

Tom1 wrote on Wed, 16 January 2019 08:54

Good morning Mirek,

Removing "thread__" from "thread__ CharEntry fc_cache[512];" fixes it.

Removing "thread__" from "thread__ FontEntry fi_cache[63];" does not have any effect on the problem.

Best regards,

Tom

BTW: Why should the caches be thread_local anyway? Isn't it true that Painter will use the same fonts with a whole bunch of threads when painting in MT? Having them common for all threads would have the used fonts cached after first hit and available to all threads. (Well, I'm likely missing something important here, but you'll probably educate me on that shortly...)

Only to avoid confusion, how the code for CharEntry exactly looks now for you? And again, the rest of the code is absolutely the same as trunk?

Mirek

I refreshed from trunk this morning and the only change is in Font.cpp:

CharEntry fc_cache[512];

Instead of original:

thread__ CharEntry fc_cache[512];

BR, Tom

Tom1 wrote on Wed, 16 January 2019 08:54

BTW: Why should the caches be thread_local anyway? Isn't it true that Painter will use the same fonts with a whole bunch of threads when painting in MT? Having them common for all threads would have the used fonts cached after first hit and available to all threads. (Well, I'm likely missing something important here, but you'll probably educate me on that shortly...)

These functions are not Painter specific. They are used for rendering text with Draw as well. thread_local is used to avoid locking the mutex for each glyphinfo query (e.g. Font fnt = Arial(20); int char_width = fnt['A']; - operator[] is using this cache)

Mirek

Tom1 wrote on Wed, 16 January 2019 09:37

mirek wrote on Wed, 16 January 2019 10:18

Tom1 wrote on Wed, 16 January 2019 08:54

Good morning Mirek,

Removing "thread__" from "thread__ CharEntry fc_cache[512];" fixes it.

Removing "thread__" from "thread__ FontEntry fi_cache[63];" does not have any effect on the problem.

Best regards,

Tom

BTW: Why should the caches be thread_local anyway? Isn't it true that Painter will use the same fonts with a whole bunch of threads when painting in MT? Having them common for all threads would have the used fonts cached after first hit and available to all threads. (Well, I'm likely missing something important here, but you'll probably educate me on that shortly...)

Only to avoid confusion, how the code for CharEntry exactly looks now for you? And again, the rest of the code is absolutely the same as trunk?

Mirek

I refreshed from trunk this morning and the only change is in Font.cpp:

CharEntry fc_cache[512];

Instead of original:

thread__ CharEntry fc_cache[512];

BR, Tom

Well,there should be additional INTERLOCKED (to replace thread__)... Anyway, that is probably not relevant..

Any chance that you see a bug there in the code? I see nothing

Can you try this (with thread__ in place)

void PaintCharacter(Painter& sw, const Pointf& p, int chr, Font font)
{
	GlyphInfo gi = GetGlyphInfo(font, chr);
	PaintCharPath pw;
	pw.sw = &sw;
	if(gi.IsNormal())
		font.Render(pw, p.x, p.y, chr);
	else
	if(gi.IsReplaced()) {
		Font fnt = font;
		fnt.Face(gi.lspc);
		fnt.Height(gi.rspc);
		fnt.Render(pw, p.x, p.y + font.GetAscent() - fnt.GetAscent(), chr);
		DLOG("Replaced " << font << " -> " << fnt << ", " << chr << " " << (char)chr);
	}
	else
	if(gi.IsComposed()) {
		DLOG("Composed " << font << ", " << chr << " " << (char)chr);
		ComposedGlyph cg;
		Compose(font, chr, cg);
		font.Render(pw, p.x, p.y, cg.basic_char);
		sw.Div();
		cg.mark_font.Render(pw, p.x + cg.mark_pos.x, p.y + cg.mark_pos.y, cg.mark_char);
	}
	sw.EvenOdd(true);
}

bring it to error and check whether there is anything in the log?

Mirek

Even this fails:

//CharEntry fc_cache[512];

GlyphInfo GetGlyphInfo(Font font, int chr)
{
	//Mutex::Lock __(sFontLock);
	GlyphInfo info;
	INTERLOCKED{
	font.RealizeStd();
	info=GetGlyphInfoSys(font, chr);
	}
	return info;
}

BR, Tom

...

Well, those returns some sanity...

I have found possible issue and commited to trunk, please plain trunk now.. The chance that this fixes the issue is like 5%, but it is worth trying....

What about this:

HFONT GetWin32Font(Font fnt, int angle)
{
	LTIMING("GetWin32Font");
	static HFontEntry cache[FONTCACHE];
	ONCELOCK {
		for(int i = 0; i < FONTCACHE; i++)
			cache[i].font.Height(-30000);
	}
	HFontEntry be;
	be = cache[0];
/*	for(int i = 0; i < FONTCACHE; i++) {
		HFontEntry e = cache[i];
		if(i)
			cache[i] = be;
		if(e.font == fnt && e.angle == angle) {
			if(i)
				cache[0] = e;
			return e.hfont;
		}
		be = e;
	}*/
	LTIMING("GetWin32Font2");
	if(be.hfont)
		DeleteObject(be.hfont);

	be.font = fnt;
	be.angle = angle;
#ifdef PLATFORM_WINCE
	LOGFONT lfnt;
	Zero(lfnt);
	lfnt.lfHeight = fnt.GetHeight() ? -abs(fnt.GetHeight()) : -12;
	lfnt.lfWeight = fnt.IsBold() ? FW_BOLD : FW_NORMAL;
	lfnt.lfItalic = fnt.IsItalic();
	lfnt.lfUnderline = fnt.IsUnderline();
	lfnt.lfStrikeOut = fnt.IsStrikeout();
	wcscpy(lfnt.lfFaceName, ToSystemCharset(fnt.GetFaceName()));
	be.hfont = CreateFontIndirect(&lfnt);
#else
	be.hfont = CreateFont(
		fnt.GetHeight() ? -abs(fnt.GetHeight()) : -12,
		fnt.GetWidth(), angle, angle, fnt.IsBold() ? FW_BOLD : FW_NORMAL,
		fnt.IsItalic(), fnt.IsUnderline(), fnt.IsStrikeout(),
		fnt.GetFace() == Font::SYMBOL ? SYMBOL_CHARSET : DEFAULT_CHARSET,
		fnt.IsTrueTypeOnly() ? OUT_TT_ONLY_PRECIS : OUT_DEFAULT_PRECIS,
		CLIP_DEFAULT_PRECIS,
		fnt.IsNonAntiAliased() ? NONANTIALIASED_QUALITY : DEFAULT_QUALITY,
		DEFAULT_PITCH|FF_DONTCARE,
		fnt.GetFaceName()
	);
#endif
	cache[0] = be;
	return be.hfont;
}

(It is actually possible that this will INCREASE the number of wrong letters, even that would be important to know)

How about

HFONT GetWin32Font(Font fnt, int angle)
{
	LTIMING("GetWin32Font");
	static HFontEntry cache[FONTCACHE];
	ONCELOCK {
		for(int i = 0; i < FONTCACHE; i++)
			cache[i].font.Height(-30000);
	}
	HFontEntry be;
	be = cache[0];
INTERLOCKED
	for(int i = 0; i < FONTCACHE; i++) {
		HFontEntry e = cache[i];
		if(i)
			cache[i] = be;
		if(e.font == fnt && e.angle == angle) {
			if(i)
				cache[0] = e;
			return e.hfont;
		}
		be = e;
	}
	LTIMING("GetWin32Font2");
	if(be.hfont)
		DeleteObject(be.hfont);

	be.font = fnt;
	be.angle = angle;
#ifdef PLATFORM_WINCE
	LOGFONT lfnt;
	Zero(lfnt);
	lfnt.lfHeight = fnt.GetHeight() ? -abs(fnt.GetHeight()) : -12;
	lfnt.lfWeight = fnt.IsBold() ? FW_BOLD : FW_NORMAL;
	lfnt.lfItalic = fnt.IsItalic();
	lfnt.lfUnderline = fnt.IsUnderline();
	lfnt.lfStrikeOut = fnt.IsStrikeout();
	wcscpy(lfnt.lfFaceName, ToSystemCharset(fnt.GetFaceName()));
	be.hfont = CreateFontIndirect(&lfnt);
#else
	be.hfont = CreateFont(
		fnt.GetHeight() ? -abs(fnt.GetHeight()) : -12,
		fnt.GetWidth(), angle, angle, fnt.IsBold() ? FW_BOLD : FW_NORMAL,
		fnt.IsItalic(), fnt.IsUnderline(), fnt.IsStrikeout(),
		fnt.GetFace() == Font::SYMBOL ? SYMBOL_CHARSET : DEFAULT_CHARSET,
		fnt.IsTrueTypeOnly() ? OUT_TT_ONLY_PRECIS : OUT_DEFAULT_PRECIS,
		CLIP_DEFAULT_PRECIS,
		fnt.IsNonAntiAliased() ? NONANTIALIASED_QUALITY : DEFAULT_QUALITY,
		DEFAULT_PITCH|FF_DONTCARE,
		fnt.GetFaceName()
	);
#endif
	cache[0] = be;
	return be.hfont;
}