After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 541608 - Different colors for combining marks and base glyphs
Different colors for combining marks and base glyphs
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: pango-maint
pango-maint
Depends on:
Blocks:
 
 
Reported: 2008-07-04 21:48 UTC by Khaled Hosny
Modified: 2015-09-03 03:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The output of pango-view using the mentioned sample markup (7.25 KB, image/png)
2008-07-04 21:49 UTC, Khaled Hosny
  Details
Test patch (726 bytes, patch)
2015-07-26 21:48 UTC, Khaled Hosny
none Details | Review
Test document (282 bytes, text/plain)
2015-07-26 21:48 UTC, Khaled Hosny
  Details
Test output (17.00 KB, image/png)
2015-07-26 21:49 UTC, Khaled Hosny
  Details
Try to use grapheme boundaries instead of clusters for justification (2.50 KB, patch)
2015-07-27 22:59 UTC, Khaled Hosny
none Details | Review
Test output after the second patch (16.97 KB, image/png)
2015-07-27 23:00 UTC, Khaled Hosny
  Details
Use finer HarfBuzz cluster level (1.15 KB, patch)
2015-08-18 14:41 UTC, Khaled Hosny
committed Details | Review
Use grapheme boundaries in justify_clusters() (3.95 KB, patch)
2015-08-18 14:41 UTC, Khaled Hosny
committed Details | Review

Description Khaled Hosny 2008-07-04 21:48:32 UTC
Pango doesn't seem to support using different colors for combining marks and base glyphs.
The following markup should give a green base word with red diacritics, but instead, the base glyph takes the same color as the mark.

{{{
<span color="green">خ<span color="red">َ</span>ال<span color="red">ِ</span>د<span color="red">ْ</span></span>
}}}
Comment 1 Khaled Hosny 2008-07-04 21:49:57 UTC
Created attachment 114007 [details]
The output of pango-view using the mentioned sample markup
Comment 2 Behdad Esfahbod 2013-09-10 21:26:18 UTC
Yeah, known HarfBuzz issue.  Will work on it.
Comment 3 Behdad Esfahbod 2015-07-26 20:29:07 UTC
This should now be possible thanks to:
https://github.com/behdad/harfbuzz/commit/376d587f36b4ff10342ee6ca3bacd73532ea44c8#commitcomment-12362295

I did a quick test setting cluster_level to 1, it didn't change the behaviour of the test.  Need to debug.
Comment 4 Khaled Hosny 2015-07-26 21:48:19 UTC
Created attachment 308186 [details] [review]
Test patch

Setting cluster level to 1 works fine here as far as colouring combining marks is concerned, but it breaks is_cluster_start attribute (at least for combining marks) which is used in justify_clusters() to skip expansion inside clusters, but after this change it will see combining marks as separate clusters and will try to expand them causing a shift in the mark position.
Comment 5 Khaled Hosny 2015-07-26 21:48:46 UTC
Created attachment 308187 [details]
Test document
Comment 6 Khaled Hosny 2015-07-26 21:49:07 UTC
Created attachment 308188 [details]
Test output
Comment 7 Behdad Esfahbod 2015-07-26 21:51:18 UTC
Thanks!  Will take a look.
Comment 8 Khaled Hosny 2015-07-26 21:52:08 UTC
The mark expansion issue does not happen in Arabic, presumably because no inter character spacing is used to justify Arabic.
Comment 9 Khaled Hosny 2015-07-27 22:59:37 UTC
Created attachment 308265 [details] [review]
Try to use grapheme boundaries instead of clusters for justification

Here is a test patch that tries to use grapheme boundaries instead of clusters during justification. It seems to work, at least with the example I posted.
Comment 10 Khaled Hosny 2015-07-27 23:00:52 UTC
Created attachment 308266 [details]
Test output after the second patch
Comment 11 Behdad Esfahbod 2015-08-18 11:59:40 UTC
Khaled, feel like landing this after adding HarfBuzz version check?  Version should check for (not-released-yet) harfbuzz >= 1.0.2.  Or we can just require that version when it's released.  Thanks.
Comment 12 Khaled Hosny 2015-08-18 14:41:25 UTC
The following fixes have been pushed:
3ea523f Use finer HarfBuzz cluster level
25976af Use grapheme boundaries in justify_clusters()

I used a build time check for HarfBuzz 1.0.2, once it is released we can consider making it a hard requirement.
Comment 13 Khaled Hosny 2015-08-18 14:41:31 UTC
Created attachment 309477 [details] [review]
Use finer HarfBuzz cluster level

So that we can set attributes (e.g. colors) to the marks and their bases
indecently.
Comment 14 Khaled Hosny 2015-08-18 14:41:38 UTC
Created attachment 309478 [details] [review]
Use grapheme boundaries in justify_clusters()

Instead of relying on the fact that marks have the same cluster number
as their bases, which will change in the next commit.
Comment 15 Matthias Clasen 2015-08-31 23:52:49 UTC
Reopening.

The "Use finer HarfBuzz cluster level" causes the testiter test to segfault.

(gdb) bt
  • #0 raise
    from /lib64/libc.so.6
  • #1 abort
    from /lib64/libc.so.6
  • #2 __libc_message
    from /lib64/libc.so.6
  • #3 _int_free
    from /lib64/libc.so.6
  • #4 free
    from /lib64/libc.so.6
  • #5 g_free
    at gmem.c line 189
  • #6 insert_run
    at pango-layout.c line 3344
  • #7 process_item
    at pango-layout.c line 3449
  • #8 process_line
    at pango-layout.c line 3708
  • #9 pango_layout_check_lines
    at pango-layout.c line 4036
  • #10 pango_layout_get_iter
    at pango-layout.c line 5705
  • #11 iter_char_test
    at testiter.c line 94
  • #12 test_layout_iter
    at testiter.c line 231
  • #13 test_case_run
    at gtestutils.c line 2124
  • #14 g_test_run_suite_internal
    at gtestutils.c line 2185
  • #15 g_test_run_suite_internal
    at gtestutils.c line 2196
  • #16 g_test_run_suite
    at gtestutils.c line 2249
  • #17 g_test_run
    at gtestutils.c line 1553
  • #18 main
    at testiter.c line 297

Comment 16 Khaled Hosny 2015-09-01 00:35:42 UTC
I can’t reproduce the crash, make check passes just fine. I even ran testiter under valgrind and all is clean.
Comment 17 Matthias Clasen 2015-09-01 01:14:52 UTC
Here is what valgrind tells me:

==25456== Invalid write of size 4
==25456==    at 0x4E52839: pango_glyph_item_get_logical_widths (pango-glyph-item.c:823)
==25456==    by 0x4E551F2: process_item (pango-layout.c:3465)
==25456==    by 0x4E579FE: process_line (pango-layout.c:3708)
==25456==    by 0x4E579FE: pango_layout_check_lines (pango-layout.c:4036)
==25456==    by 0x4E5A11F: pango_layout_get_iter (pango-layout.c:5705)
==25456==    by 0x40154D: iter_char_test (testiter.c:94)
==25456==    by 0x40154D: test_layout_iter (testiter.c:231)
==25456==    by 0x713E982: test_case_run (gtestutils.c:2124)
==25456==    by 0x713E982: g_test_run_suite_internal (gtestutils.c:2185)
==25456==    by 0x713EB4A: g_test_run_suite_internal (gtestutils.c:2196)
==25456==    by 0x713EEAA: g_test_run_suite (gtestutils.c:2249)
==25456==    by 0x713EEE0: g_test_run (gtestutils.c:1553)
==25456==    by 0x4011FF: main (testiter.c:297)
==25456==  Address 0xcecb3fc is 0 bytes after a block of size 60 alloc'd
==25456==    at 0x4C2BC50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25456==    by 0x711F0C8: g_malloc (gmem.c:94)
==25456==    by 0x4E551DD: process_item (pango-layout.c:3464)
==25456==    by 0x4E579FE: process_line (pango-layout.c:3708)
==25456==    by 0x4E579FE: pango_layout_check_lines (pango-layout.c:4036)
==25456==    by 0x4E5A11F: pango_layout_get_iter (pango-layout.c:5705)
==25456==    by 0x40154D: iter_char_test (testiter.c:94)
==25456==    by 0x40154D: test_layout_iter (testiter.c:231)
==25456==    by 0x713E982: test_case_run (gtestutils.c:2124)
==25456==    by 0x713E982: g_test_run_suite_internal (gtestutils.c:2185)
==25456==    by 0x713EB4A: g_test_run_suite_internal (gtestutils.c:2196)
==25456==    by 0x713EEAA: g_test_run_suite (gtestutils.c:2249)
==25456==    by 0x713EEE0: g_test_run (gtestutils.c:1553)
==25456==    by 0x4011FF: main (testiter.c:297)
==25456==
Comment 18 Matthias Clasen 2015-09-01 02:19:29 UTC
Adding 

g_print ("glyph item num_chars %d, iter start_char %d end_char %d\n",
         glyph_item->item->num_chars, iter.start_char, iter.end_char);

in the loop in pango_glyph_item_get_logical_widths yields:

glyph item num_chars 15, iter start_char 0 end_char 1
glyph item num_chars 15, iter start_char 1 end_char 2
glyph item num_chars 15, iter start_char 2 end_char 3
glyph item num_chars 15, iter start_char 3 end_char 4
glyph item num_chars 15, iter start_char 4 end_char 5
glyph item num_chars 15, iter start_char 5 end_char 6
glyph item num_chars 15, iter start_char 6 end_char 7
glyph item num_chars 15, iter start_char 7 end_char 8
glyph item num_chars 15, iter start_char 8 end_char 9
glyph item num_chars 15, iter start_char 9 end_char 10
glyph item num_chars 15, iter start_char 10 end_char 11
glyph item num_chars 15, iter start_char 11 end_char 12
glyph item num_chars 15, iter start_char 12 end_char 13
glyph item num_chars 15, iter start_char 13 end_char 14
glyph item num_chars 15, iter start_char 14 end_char 15
glyph item num_chars 2, iter start_char 0 end_char 1
glyph item num_chars 2, iter start_char 1 end_char 2
glyph item num_chars 15, iter start_char 0 end_char 1
glyph item num_chars 15, iter start_char 1 end_char 2
glyph item num_chars 15, iter start_char 2 end_char 3
glyph item num_chars 15, iter start_char 3 end_char 4
glyph item num_chars 15, iter start_char 4 end_char 5
glyph item num_chars 15, iter start_char 5 end_char 6
glyph item num_chars 15, iter start_char 6 end_char 7
glyph item num_chars 15, iter start_char 7 end_char 8
glyph item num_chars 15, iter start_char 8 end_char 9
glyph item num_chars 15, iter start_char 9 end_char 10
glyph item num_chars 15, iter start_char 10 end_char 11
glyph item num_chars 15, iter start_char 11 end_char 12
glyph item num_chars 15, iter start_char 12 end_char 13
glyph item num_chars 15, iter start_char 13 end_char 14
glyph item num_chars 15, iter start_char 14 end_char 15
glyph item num_chars 15, iter start_char 0 end_char 1
glyph item num_chars 15, iter start_char 1 end_char 2
glyph item num_chars 15, iter start_char 2 end_char 3
glyph item num_chars 15, iter start_char 3 end_char 4
glyph item num_chars 15, iter start_char 4 end_char 5
glyph item num_chars 15, iter start_char 5 end_char 6
glyph item num_chars 15, iter start_char 6 end_char 8
glyph item num_chars 15, iter start_char 6 end_char 8
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 8 end_char 30
glyph item num_chars 15, iter start_char 30 end_char 32
glyph item num_chars 15, iter start_char 30 end_char 32
glyph item num_chars 15, iter start_char 32 end_char 33
glyph item num_chars 15, iter start_char 33 end_char 34
glyph item num_chars 15, iter start_char 34 end_char 35
glyph item num_chars 15, iter start_char 35 end_char 36
glyph item num_chars 15, iter start_char 36 end_char 37

The last few lines here point at the trouble: start/end_char are clearly outside of the allocated range [0,15] of the logical_widths array
Comment 19 Matthias Clasen 2015-09-01 03:53:40 UTC
The failure depends on font configuration, I think.

I can make the crash go away here if I add

  font_desc = pango_font_description_from_string ("monospace");
  pango_context_set_font_description (context, font_desc);

I do get the crash with e.g "cantarell 11"
Comment 20 Matthias Clasen 2015-09-01 03:55:39 UTC
Here are some (imo correct) assertions that will trigger for my bad cases:


diff --git a/pango/pango-glyph-item.c b/pango/pango-glyph-item.c
index 6954ed8..cb55998 100644
--- a/pango/pango-glyph-item.c
+++ b/pango/pango-glyph-item.c
@@ -315,6 +315,10 @@ pango_glyph_item_iter_next_cluster (PangoGlyphItemIter *iter)
     }
 
   iter->end_glyph = glyph_index;
+
+  g_assert (iter->start_char < iter->end_char);
+  g_assert (iter->end_char <= item->num_chars);
+
   return TRUE;
 }
Comment 21 Behdad Esfahbod 2015-09-01 12:57:15 UTC
Ok, this is a bug in harfbuzz cluster-level=1.  Fixing.
Comment 22 Behdad Esfahbod 2015-09-01 15:35:04 UTC
Ok, HarfBuzz fixes are in...
Comment 23 Behdad Esfahbod 2015-09-01 16:05:04 UTC
Ok, harfbuzz-1.0.3 should fix this.