Merge branch 'line-breaking-fixes3' into 'main'

More line breaking fixes

See merge request GNOME/pango!507

2 files changed, 78 insertions, 38 deletions

diff --git a/pango/pango-layout.c b/pango/pango-layout.c
index 5cdaa531..471cbb64 100644
--- a/pango/pango-layout.c
+++ b/pango/pango-layout.c
@@ -3872,16 +3872,27 @@ process_item (PangoLayout     *layout,
   if ((width <= state->remaining_width || (item->num_chars == 1 && !line->runs)) &&
       !no_break_at_end)
     {
-      state->remaining_width -= width;
-      state->remaining_width = MAX (state->remaining_width, 0);
-      insert_run (line, state, item, TRUE);
+      insert_run (line, state, item, FALSE);
+      width = pango_glyph_string_get_width (((PangoGlyphItem *)(line->runs->data))->glyphs);
 
-      return BREAK_ALL_FIT;
+      if (width <= state->remaining_width || (item->num_chars == 1 && !line->runs))
+        {
+          state->remaining_width -= width;
+          state->remaining_width = MAX (state->remaining_width, 0);
+
+          pango_glyph_string_free (state->glyphs);
+          state->glyphs = NULL;
+
+          return BREAK_ALL_FIT;
+        }
+
+      /* if it doesn't fit after shaping, revert and proceed to break the item */
+      uninsert_run (line);
     }
-  else
+
     {
-      int num_chars = item->num_chars;
-      int break_num_chars = num_chars;
+      int num_chars;
+      int break_num_chars = item->num_chars;
       int break_width = width;
       int orig_width = width;
       int break_extra_width = 0;
@@ -3918,33 +3929,62 @@ process_item (PangoLayout     *layout,
           /* check one more time if the whole item fits after removing the space */
           if (width + break_extra_width <= state->remaining_width && !no_break_at_end)
             {
-              state->remaining_width -= width + break_extra_width;
-              state->remaining_width = MAX (state->remaining_width, 0);
-              insert_run (line, state, item, TRUE);
+              insert_run (line, state, item, FALSE);
+              width = pango_glyph_string_get_width (((PangoGlyphItem *)(line->runs->data))->glyphs);
 
-              return BREAK_ALL_FIT;
+              if (width + break_extra_width <= state->remaining_width)
+                {
+                  state->remaining_width -= width + break_extra_width;
+                  state->remaining_width = MAX (state->remaining_width, 0);
+
+                  pango_glyph_string_free (state->glyphs);
+                  state->glyphs = NULL;
+
+                  return BREAK_ALL_FIT;
+                }
+
+              uninsert_run (line);
             }
         }
 
       /* See how much of the item we can stuff in the line. */
       width = 0;
-      extra_width = 0;
 
       for (num_chars = 0; num_chars < item->num_chars; num_chars++)
         {
           extra_width = find_break_extra_width (layout, state, num_chars);
 
-          if (width + extra_width > state->remaining_width && break_num_chars < item->num_chars)
-            break;
+          /* We don't want to walk the entire item if we can help it, but
+           * we need to keep going at least until we've found a breakpoint
+           * that 'works' (as in, it doesn't overflow the budget we have,
+           * or there is no hope of finding a better one).
+           *
+           * We rely on the fact that MIN(width + extra_width, width) is
+           * monotonically increasing.
+           */
+          if (MIN (width + extra_width, width) > state->remaining_width &&
+              break_num_chars < item->num_chars &&
+              (break_width + break_extra_width <= state->remaining_width ||
+               MIN (width + extra_width, width) > break_width + break_extra_width))
+            {
+              break;
+            }
 
           /* If there are no previous runs we have to take care to grab at least one char. */
           if (!break_disabled[num_chars] &&
               can_break_at (layout, state->start_offset + num_chars, retrying_with_char_breaks) &&
               (num_chars > 0 || line->runs))
             {
-              break_num_chars = num_chars;
-              break_width = width;
-              break_extra_width = extra_width;
+              /* If we had a breakpoint already, we only want to replace it with a better one. */
+              if (width + extra_width <= state->remaining_width ||
+                  width + extra_width < break_width + break_extra_width ||
+                  (width + extra_width == break_width + break_extra_width &&
+                   num_chars > break_num_chars))
+                {
+                  break_num_chars = num_chars;
+                  break_width = width;
+                  break_extra_width = extra_width;
+                }
             }
 
           width += state->log_widths[state->log_widths_offset + num_chars];
@@ -3953,9 +3993,8 @@ process_item (PangoLayout     *layout,
       if (layout->wrap == PANGO_WRAP_WORD_CHAR && force_fit && break_width + break_extra_width > state->remaining_width && !retrying_with_char_breaks)
         {
           retrying_with_char_breaks = TRUE;
-          num_chars = item->num_chars;
+          break_num_chars = item->num_chars;
           width = orig_width;
-          break_num_chars = num_chars;
           break_width = width;
           goto retry_break;
         }
@@ -3992,6 +4031,8 @@ process_item (PangoLayout     *layout,
 
               if (break_needs_hyphen (layout, state, break_num_chars))
                 new_item->analysis.flags |= PANGO_ANALYSIS_FLAG_NEED_HYPHEN;
+              else
+                new_item->analysis.flags &= ~PANGO_ANALYSIS_FLAG_NEED_HYPHEN;
 
               /* Add the width back, to the line, reshape, subtract the new width */
               state->remaining_width = remaining;
@@ -3999,27 +4040,26 @@ process_item (PangoLayout     *layout,
 
               break_width = pango_glyph_string_get_width (((PangoGlyphItem *)(line->runs->data))->glyphs);
 
-              if (state->start_offset + break_num_chars > 0 &&
-                  layout->log_attrs[state->start_offset + break_num_chars - 1].is_white)
-                break_width -= state->log_widths[state->log_widths_offset + break_num_chars - 1];
+              /* After the shaping, break_width includes a possible hyphen.
+               * We subtract break_extra_width to account for that.
+               */
+              if (new_item->analysis.flags & PANGO_ANALYSIS_FLAG_NEED_HYPHEN)
+                break_width -= break_extra_width;
 
-              if (break_width > state->remaining_width &&
-                  !break_disabled[break_num_chars] &&
-                  break_num_chars > 1)
+              if (break_width + break_extra_width > state->remaining_width &&
+                  !break_disabled[break_num_chars])
                 {
-                  /* Unsplit the item, disable the breakpoint, try again */
-
+                  /* Unsplit the item, disable the breakpoint, try find a better one.
+                   *
+                   * If we can't find a different breakpoint that works better, we'll
+                   * end up here again, with break_disabled being set, and take the break
+                   */
                   uninsert_run (line);
                   pango_item_free (new_item);
                   pango_item_unsplit (item, length, break_num_chars);
 
                   break_disabled[break_num_chars] = TRUE;
 
-                  num_chars = item->num_chars;
-                  width = orig_width;
-                  break_num_chars = num_chars;
-                  break_width = width;
-
                   goto retry_break;
                 }
 
diff --git a/tests/layouts/valid-5.expected b/tests/layouts/valid-5.expected
index ce4ff215..82f413c9 100644
--- a/tests/layouts/valid-5.expected
+++ b/tests/layouts/valid-5.expected
@@ -21,14 +21,14 @@ range 31 2147483647
 
 --- cursor positions
 
-0(0) 1(0) 2(0) 3(0) 4(0) 5(0) 6(0) 7(0) 8(0) 9(0) 10(0) 11(0) 12(0) 13(0) 14(0) 15(0) 16(0) 17(0) 18(0) 19(0) 20(0) 20(1) 22(0) 23(0) 24(0) 25(0) 26(0) 27(0) 28(0) 29(0) 30(0) 31(0) 32(0) 33(0) 34(0) 35(0) 36(0) 37(0) 38(0) 39(0) 40(0) 41(0) 42(0) 43(0) 43(1) 45(0) 46(0) 47(0) 48(0) 49(0) 50(0) 51(0) 52(0) 53(0) 54(0) 55(0) 56(0) 57(0) 58(0) 59(0) 60(0) 61(0) 62(0) 63(0) 64(0) 65(0) 66(0) 67(0) 67(1) 69(0) 70(0) 71(0) 72(0) 73(0) 74(0) 75(0) 76(0) 77(0) 78(0) 79(0) 80(0) 81(0) 82(0) 83(0) 84(0) 85(0) 86(0) 87(0) 88(0) 89(0) 89(1) 91(0) 92(0) 93(0) 94(0) 95(0) 96(0) 97(0) 98(0) 99(0) 100(0) 101(0) 102(0) 103(0) 104(0) 105(0) 106(0) 107(0) 108(0) 109(0) 110(0) 111(0) 112(0) 113(0) 114(0) 115(0) 116(0) 116(1) 118(0) 119(0) 120(0) 121(0) 122(0) 123(0) 123(1) 125(0) 
+0(0) 1(0) 2(0) 3(0) 4(0) 5(0) 6(0) 7(0) 8(0) 9(0) 10(0) 11(0) 12(0) 13(0) 14(0) 15(0) 16(0) 17(0) 18(0) 19(0) 20(0) 20(1) 22(0) 23(0) 24(0) 25(0) 26(0) 27(0) 28(0) 29(0) 30(0) 31(0) 32(0) 33(0) 34(0) 35(0) 36(0) 37(0) 38(0) 39(0) 40(0) 41(0) 42(0) 43(0) 43(1) 45(0) 46(0) 47(0) 48(0) 49(0) 50(0) 51(0) 52(0) 53(0) 54(0) 55(0) 56(0) 57(0) 58(0) 59(0) 60(0) 61(0) 62(0) 63(0) 64(0) 65(0) 66(0) 67(0) 68(0) 69(0) 70(0) 71(0) 72(0) 73(0) 73(1) 75(0) 76(0) 77(0) 78(0) 79(0) 80(0) 81(0) 82(0) 83(0) 84(0) 85(0) 86(0) 87(0) 88(0) 89(0) 89(1) 91(0) 92(0) 93(0) 94(0) 95(0) 96(0) 97(0) 98(0) 99(0) 100(0) 101(0) 102(0) 103(0) 104(0) 105(0) 106(0) 107(0) 108(0) 109(0) 110(0) 111(0) 112(0) 113(0) 114(0) 115(0) 116(0) 116(1) 118(0) 119(0) 120(0) 121(0) 122(0) 123(0) 123(1) 125(0) 
 
 --- lines
 
 i=1, index=0, paragraph-start=1, dir=ltr 'A test with multiple '
 i=2, index=21, paragraph-start=0, dir=ltr 'paragraphs and with no-'
-i=3, index=44, paragraph-start=0, dir=ltr 'break attributes, which '
-i=4, index=68, paragraph-start=0, dir=ltr 'might trigger a crash.
+i=3, index=44, paragraph-start=0, dir=ltr 'break attributes, which might '
+i=4, index=74, paragraph-start=0, dir=ltr 'trigger a crash.
 '
 i=5, index=91, paragraph-start=1, dir=ltr 'If it doesn't the fix has '
 i=6, index=117, paragraph-start=0, dir=ltr 'worked.
@@ -43,10 +43,10 @@ i=2, index=21, no run, line end
 i=3, index=21, chars=23, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'paragraphs and with no-'
 [21,31]font-features=tnum=1
 i=4, index=44, no run, line end
-i=5, index=44, chars=24, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'break attributes, which '
+i=5, index=44, chars=30, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'break attributes, which might '
 [21,31]font-features=tnum=1
-i=6, index=68, no run, line end
-i=7, index=68, chars=22, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'might trigger a crash.'
+i=6, index=74, no run, line end
+i=7, index=74, chars=16, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'trigger a crash.'
 [21,31]font-features=tnum=1
 i=8, index=90, no run, line end
 i=9, index=91, chars=26, level=0, gravity=south, flags=0, font=OMITTED, script=latin, language=en-us, 'If it doesn't the fix has '