I think there is a bug in get_shaper_and_font() in pango-1.8.2

[Stephen Blackheath <stephen blacksapphire com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dear Pango developers,

Hello there!  I've been trying to work on getting Mozilla Firefox to
render Hebrew properly, and I have discovered what I think is a bug in
Pango 1.8.2, and - (without a thorough check), seems to exist in
pango-1.9.1 as well.  The information below doesn't constitute a
"proper" bug report as such, but it should hopefully at least be enough
to point out that there is a problem.  Please let me know if you want
more information, or a proper test case.

I have attached a *very rough* patch to this email that fixes the
symptom of the problem for me.

Here is how to reproduce the bug on Firefox/DeerPark with my "4.7 patch"
applied on Linux (see http://www.blacksapphire.com/firefox-rtl/ for the
background on this patch):

1. Go to this URL:
http://www.mechon-mamre.org/c/ct/c0101.htm

2. Open a new tab.

3. In the new tab, go to:
http://verboomen.starline-inc.de/d11.php2xml?sfr=1&prq=1

4. Click back to the first tab.

5. Click back to the second tab.

6. Click back to the first tab.  (I don't know why you have to switch
back twice.)

7. Now you will see that the vowels have shifted over to be located *in
between* the letters instead of under them as they should be.
Screenshot at http://blacksapphire.com/firefox-rtl/bug21-screenshot.png

Here is why the bug happens:
* I don't know why, but Pango classifies Hebrew vowels as "inherited"
script instead of Hebrew, while the letters themselves are Hebrew.  This
means that in the context of a language other than Hebrew, they will be
rendered using the "basic" shaper engine instead of Dov's Hebrew one.

* The engine to use for a given character is cached, but the cache
implementation doesn't look quite right.  This happens in a function
called get_shaper_and_font().  It uses only the character value as the
cache key, but the engine is chosen on the basis of more information
than just the character value.  Since the cache key does not match the
criteria for the choice of the value cached, the cache can return
different things to what the underlying algorithm would return without
caching. The underlying algorithm appears to work.  It seems the wrongly
implemented cache is causing the bug.


Steve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFC+Xy34nWub/+PLZsRArC8AKCHck49fBwxS6Nks0QXwpYxDd+oKQCbBbvd
UfHaNaEDYeXB3ldRQ/IssG8=
=FTt1
-----END PGP SIGNATURE-----

--- pango-1.8.2.orig/pango/pango-context.c	2005-06-15 07:54:01.000000000 +1200
+++ pango-1.8.2/pango/pango-context.c	2005-08-07 21:45:46.137016112 +1200
@@ -479,12 +479,14 @@
 static gboolean
 shaper_font_cache_get (ShaperFontCache   *cache,
 		       gunichar           wc,
+                       PangoLanguage     *language,
+                       PangoScript        script,
 		       PangoEngineShape **shape_engine,
 		       PangoFont        **font)
 {
   ShaperFontElement *element;
   
-  element = g_hash_table_lookup (cache->hash, GUINT_TO_POINTER (wc));
+  element = g_hash_table_lookup (cache->hash, GUINT_TO_POINTER (wc+(guint)language+(guint)script*0x10000));
   if (element)
     {
       *shape_engine = element->shape_engine;
@@ -499,6 +501,8 @@
 static void
 shaper_font_cache_insert (ShaperFontCache   *cache,
 			  gunichar           wc,
+                          PangoLanguage     *language,
+                          PangoScript        script,
 			  PangoEngineShape  *shape_engine,
 			  PangoFont         *font)
 {
@@ -506,7 +510,7 @@
   element->shape_engine = shape_engine;
   element->font = font;
   
-  g_hash_table_insert (cache->hash, GUINT_TO_POINTER (wc), element);
+  g_hash_table_insert (cache->hash, GUINT_TO_POINTER (wc+(guint)language+(guint)script*0x10000), element);
 }
 
 /**********************************************************************/
@@ -918,7 +922,7 @@
 
   /* We'd need a separate cache when fallback is disabled, but since lookup
    * with fallback disabled is faster anyways, we just skip caching */
-  if (state->enable_fallback && shaper_font_cache_get (state->cache, wc, shape_engine, font))
+  if (state->enable_fallback && shaper_font_cache_get (state->cache, wc, state->derived_lang, state->script, shape_engine, font))
     return *shape_engine != NULL;
 
   if (!state->exact_engines && !state->fallback_engines)
@@ -943,7 +947,7 @@
 
       /* skip caching if fallback disabled (see above) */
       if (state->enable_fallback)
-        shaper_font_cache_insert (state->cache, wc, *shape_engine, *font);
+        shaper_font_cache_insert (state->cache, wc, state->derived_lang, state->script, *shape_engine, *font);
 
       return TRUE;
     }
@@ -959,8 +963,8 @@
 
   /* skip caching if fallback disabled (see above) */
   if (state->enable_fallback)
-    shaper_font_cache_insert (state->cache, wc, *shape_engine, *font);
-      
+    shaper_font_cache_insert (state->cache, wc, state->derived_lang, state->script, *shape_engine, *font);
+
   return *shape_engine != NULL;
 }