robertknight/tex-linebreak

Break points are incorrect if items exactly fill line width

zamfofex opened this issue · 2 comments

Consider the following monospaced text: a a a a a. I can represent it as the following array:

let items =
	[
		{type: "box", width: 1},
		{type: "glue", width: 1, shrink: 0, stretch: 5},
		{type: "box", width: 1},
		{type: "glue", width: 1, shrink: 0, stretch: 5},
		{type: "box", width: 1},
		{type: "glue", width: 1, shrink: 0, stretch: 5},
		{type: "box", width: 1},
		{type: "glue", width: 1, shrink: 0, stretch: 5},
		{type: "box", width: 1},
	]

Since I wrote shrink: 0, I would expect for the glues to never be able to shrink, however, this library doesn’t seem to honor that:

console.log(breakLines(items, 3))
// Expected result: [0, 3, 7]
// Actual result: [0, 5]

From my tests, the effects of this problem are more easily visible in narrow paragraphs, and manifest in the form of too tight spacing in some lines.

(Apologies if I’m just doing something wrong entirely.)

Note: I was able to resolve my issue by adding a dummy trailing glue to the end of the array.

Thanks for the clear bug report!

It looks like the initial output ([0, 5]) occurs because the total width of the first 3 items is exactly the line width and also because there is no trailing glue to fill the final line. If I add a trailing glue and change the expression to console.log(breakLines(items, 3.01)) it produces the expected result ([0, 3, 7]). The second line includes 4 items because glue items at the start of a line are ignored.

Changing the section of the layout code that calculates how much to shrink or stretch a line by to include the case where the actual and preferred line lengths are exactly equal resolves the issue:

if (actualLen < idealLen) {

diff --git a/src/layout.ts b/src/layout.ts
index 9b663bd..af06c3b 100644
--- a/src/layout.ts
+++ b/src/layout.ts
@@ -250,9 +250,12 @@ export function breakLines(
       // nb. Division by zero produces `Infinity` here, which is what we want.
       if (actualLen < idealLen) {
         adjustmentRatio = (idealLen - actualLen) / lineStretch;
-      } else {
+      } else if (actualLen > idealLen) {
         adjustmentRatio = (idealLen - actualLen) / lineShrink;
+      } else {
+        adjustmentRatio = 0.0;
       }
+
       if (adjustmentRatio > currentMaxAdjustmentRatio) {
         // In case we need to try again later with a higher
         // `maxAdjustmentRatio`, track the minimum value needed to produce

I'm going to re-open this as I think it does count as a bug.