GNOME Bugzilla – Bug 621869
Distinct performance issues with Japanese only on win32 systems
Last modified: 2011-01-03 18:06:36 UTC
Created attachment 163903 [details] [review] Patch that fixes the slow Japanese rendering on win32. Note: This bug was initially reported on the gtk-i18n-list by David E. Hollingsworth. http://mail.gnome.org/archives/gtk-i18n-list/2009-July/msg00009.html http://mail.gnome.org/archives/gtk-i18n-list/2010-April/msg00002.html It seems Pango uses the Uniscribe script cache in a sub optimal way. Pango always create new script caches and throws them away in each call to uniscribe_shape. But in the Uniscribe docs it does not say that the cache needs to be cleared between ScriptItemize calls. This makes rendering of Japanese text very slow even for moderate text chunks. Also it does not seem that this performance issue is isolated to Japanese but for other scripts that need uniscribe as well. I think Japanese however suffers more severely as it uses space separators between the characters and that causes shape fragmentation and thus will generate a lot more calls to "uniscribe_shape". This will in turn make the caches quite useless. After some investigation (starting from Davids patch) I managed to create a patch that fix the issue without causing any problems for complex scripts. Regarding the testing of the patch the company I work for have deployed Pango patched with this fix. As it's used by our Translation editor (that is used by hundreds of translators around the globe) this patch has been tested in production for most major languages including Japanese, Chinese, Korean, Hindi, Arabic. The editor supports dynamic resize (zoom) of the document rendered so it should have also been tested to work with different fonts and sizes in the same session.
Are you sure the HFONT values are guaranteed to have meaningful varying contents only in the lower 32 bits on 64-bit Windows?
(In reply to comment #1) > Are you sure the HFONT values are guaranteed to have meaningful varying > contents only in the lower 32 bits on 64-bit Windows? In MSDN it documents that handles can be shared between 32 and 64 bit applications and that 64 bit windows only uses the lower 32 bits. http://msdn.microsoft.com/en-us/library/aa384203%28VS.85%29.aspx In a discussion regarding this I found there seems to be a consensus that unless MS wants to break win32 applications on purpose it should be safe for the foreseeable future. http://stackoverflow.com/questions/1822667/how-can-i-share-hwnd-between-32-and-64-bit-applications-in-win-x64
Review of attachment 163903 [details] [review]: Looks OK. Just some stylistic changes needed, and also I think we can as well just use the existing pango_win32_get_hfont() function, and in fact remove from that function the SelectObject() and GetTextMetric() calls. It was silly that pango_win32_get_hfont() selected the font into the evil global _pango_win32_hdc, and then some of the callers did that again... And call GetTextMetric() instead in the place where that data is used after calling pango_win32_get_hfont(). At the same time we can drop the tm_* fields from PangoWin32Font. Will attach a modified patch, and unless you object, commit that.
Created attachment 164309 [details] [review] Updated patch Updated version of the patch.
(In reply to comment #4) > Created an attachment (id=164309) [details] [review] > Updated patch > > Updated version of the patch. Thanks! Your changes looks good to me. But I do not have much knowledge about Pango internals so I cant really give any valid feedback. However one patch chunk failed (basic-win32.c.rej) to apply for the version I downloaded from GTK org. But once I applied it manually it complied without issues. I also did some quick tests with the patched Pango and the Japanese and complex languages rendering and seems to work well. Failed patch chunk - basic-win32.c.rej: ----------------------------------- *************** *** 31,36 **** #include "pangowin32.h" #ifndef PANGO_MODULE_PREFIX #define PANGO_MODULE_PREFIX _pango_basic_win32 #endif --- 31,38 ---- #include "pangowin32.h" + extern HFONT _pango_win32_font_get_hfont (PangoFont *font); + #ifndef PANGO_MODULE_PREFIX #define PANGO_MODULE_PREFIX _pango_basic_win32 #endif -----------------------------------
Patch pushed to master, will be in next stable release.
Awesome. Thanks for the fix guys.