Skip to content

Commit

Permalink
Merge pull request #492 from danfickle/robust_breaker
Browse files Browse the repository at this point in the history 
#482 #483 #491 Make word breaker testable and start writing tests.
  • Loading branch information
@danfickle
danfickle authored Jun 13, 2020
2 parents 5fb570d + 208f37d commit 9ab4c10
Show file tree
Hide file tree
Showing 12 changed files with 915 additions and 84 deletions.
37 changes: 25 additions & 12 deletions openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Breaker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ void copyTo(AppBreakOpportunity other) {
other.isSoftHyphenBreak = isSoftHyphenBreak;
}
}

public static LineBreakResult doBreakText(
LayoutContext c,
LineBreakContext context,
Expand All @@ -402,8 +402,22 @@ public static LineBreakResult doBreakText(
? style.getFloatPropertyProportionalWidth(CSSName.LETTER_SPACING, 0, c)
: 0f;

ToIntFunction<String> measurer = (str) ->
c.getTextRenderer().getWidth(c.getFontContext(), font, str);

String currentString = context.getStartSubstring();
FSTextBreaker iterator = lineBreaker.getBreaker(currentString, c.getSharedContext());
FSTextBreaker lineIterator = lineBreaker.getBreaker(currentString, c.getSharedContext());

return doBreakTextWords(currentString, context, avail, lineIterator, letterSpacing, measurer);
}

static LineBreakResult doBreakTextWords(
String currentString,
LineBreakContext context,
int avail,
FSTextBreaker iterator,
float letterSpacing,
ToIntFunction<String> measurer) {

int lastWrap = 0;

Expand All @@ -423,17 +437,17 @@ public static LineBreakResult doBreakText(
String subString = currentString.substring(current.left, current.right);
float extraSpacing = (current.right - current.left) * letterSpacing;

int normalSplitWidth = (int) (c.getTextRenderer().getWidth(
c.getFontContext(), font, subString) + extraSpacing);

int normalSplitWidth = (int) (measurer.applyAsInt(subString) + extraSpacing);

if (currentString.charAt(current.right - 1) == SOFT_HYPHEN) {
current.isSoftHyphenBreak = true;
int withTrailingHyphenSplitWidth = (int) (c.getTextRenderer().getWidth(
c.getFontContext(), font, subString + '-') +
int withTrailingHyphenSplitWidth = (int)
(measurer.applyAsInt(subString + '-') +
extraSpacing + letterSpacing);
current.withHyphenGraphicsLength = current.graphicsLength + withTrailingHyphenSplitWidth;

if (current.withHyphenGraphicsLength > avail) {
if (current.withHyphenGraphicsLength >= avail &&
current.right != currentString.length()) {
current.graphicsLength = current.withHyphenGraphicsLength;
lastWrap = current.left;
current.left = current.right;
Expand All @@ -458,8 +472,8 @@ public static LineBreakResult doBreakText(
current.copyTo(prev);
current.right = currentString.length();
float extraSpacing = (current.right - current.left) * letterSpacing;
int splitWidth = (int) (c.getTextRenderer().getWidth(
c.getFontContext(), font, currentString.substring(current.left)) + extraSpacing);
int splitWidth = (int) (measurer.applyAsInt(
currentString.substring(current.left)) + extraSpacing);
current.graphicsLength += splitWidth;
nextUnfittableSplitWidth = splitWidth;
}
Expand Down Expand Up @@ -500,8 +514,7 @@ public static LineBreakResult doBreakText(
} else if (current.left == currentString.length()) {
String text = context.getCalculatedSubstring();
float extraSpacing = text.length() * letterSpacing;
context.setWidth((int) (c.getTextRenderer().getWidth(
c.getFontContext(), font, text) + extraSpacing));
context.setWidth((int) (measurer.applyAsInt(text) + extraSpacing));
} else {
context.setWidth(current.graphicsLength);
}
Expand Down
48 changes: 38 additions & 10 deletions openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;

import org.w3c.dom.Element;

Expand All @@ -47,6 +48,7 @@
import com.openhtmltopdf.render.MarkerData;
import com.openhtmltopdf.render.StrutMetrics;
import com.openhtmltopdf.render.TextDecoration;
import com.openhtmltopdf.util.XRLog;

/**
* This class is responsible for flowing inline content into lines. Block
Expand Down Expand Up @@ -155,7 +157,9 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in
}

boolean inCharBreakingMode = false;

int troublesomeStartPosition = -1;
int troublesomeAttemptCount = 0;

do {
lbContext.reset();

Expand Down Expand Up @@ -183,14 +187,31 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in
needFirstLetter = false;
} else {
if (style.getWordWrap() != IdentValue.BREAK_WORD) {
if (!startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false)) {
StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false);
if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) {
continue;
}
} else {
boolean shouldContinue = !startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode);
StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode);
inCharBreakingMode = lbContext.isFinishedInCharBreakingMode();
if (shouldContinue) {

if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) {
continue;
} else if (result == StartInlineTextResult.RECONSUME_UNBREAKABLE_ON_NEW_LINE) {
if (troublesomeStartPosition == lbContext.getStart()) {
troublesomeAttemptCount++;
} else {
troublesomeStartPosition = lbContext.getStart();
troublesomeAttemptCount = 1;
}

if (troublesomeAttemptCount > 5) {
XRLog.general(Level.SEVERE,
"A fatal infinite loop bug was detected in the line breaking " +
"algorithm for break-word! Start-substring=[" + lbContext.getStartSubstring() + "], " +
"end=" + lbContext.getEnd());
throw new RuntimeException("Infinite loop bug in break-word line breaking algorithm!");
}
}
}
}
Expand Down Expand Up @@ -368,14 +389,20 @@ private static void startNewInlineLine(LayoutContext c, BlockBox box, int breakA
space.remainingWidth -= c.getBlockFormattingContext().getFloatDistance(c, current.line, space.remainingWidth);
}

private enum StartInlineTextResult {
RECONSUME_BELOW_FLOATS,
RECONSUME_UNBREAKABLE_ON_NEW_LINE,
LINE_FINISHED
}

/**
* Trys to consume the text in lbContext. If successful it creates an InlineText and adds it to the current inline
* layout box.
* Otherwise, if there are floats and the current line is otherwise empty, moves below float and trys again.
* Otherwise, trys again on a new line.
* @return true if the line is finished, false if we must continue
*/
private static boolean startInlineText(
private static StartInlineTextResult startInlineText(
LayoutContext c, LineBreakContext lbContext, InlineBox inlineBox,
SpaceVariables space, StateVariables current, int fit,
boolean trimmedLeadingSpace, boolean tryToBreakAnywhere) {
Expand Down Expand Up @@ -406,9 +433,9 @@ private static boolean startInlineText(
// Go back to before the troublesome unbreakable content.
lbContext.resetEnd();

// Return false so that we continue with line breaking with the new remaining width.
// Return appropriate ret val so that we continue with line breaking with the new remaining width.
// The InlineText we produced above is not used.
return false;
return StartInlineTextResult.RECONSUME_BELOW_FLOATS;
}
}

Expand Down Expand Up @@ -436,17 +463,18 @@ private static boolean startInlineText(
space.pendingLeftMBP -= marginBorderPadding;
space.remainingWidth -= marginBorderPadding;
}

// Line is finsished, consume content afterward on new line.
return StartInlineTextResult.LINE_FINISHED;
} else {
// We could not fit this text on the current line and it was unbreakable.
// So rewind to reconsume the troublesome text.
// This will be done on a new line as lbContext.isNeedsNewLine is true (see context
// of this method in layoutContent).
// The inline text object is not consumed.
lbContext.resetEnd();
return StartInlineTextResult.RECONSUME_UNBREAKABLE_ON_NEW_LINE;
}

// We should go ahead and create a new line if needed, after this method.
return true;
}

private static void startFirstLetterInlineLayoutBox(LayoutContext c, SpaceVariables space, StateVariables current,
Expand Down
2 changes: 1 addition & 1 deletion openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LineBreakContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public boolean isEndsOnSoftHyphen() {
}

public void setEndsOnSoftHyphen(boolean b) {
this._endsOnSoftHyphen = true;
this._endsOnSoftHyphen = b;
}

/**
Expand Down
63 changes: 3 additions & 60 deletions openhtmltopdf-core/src/test/java/com/openhtmltopdf/layout/BreakerTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,72 +1,15 @@
package com.openhtmltopdf.layout;

import java.util.function.ToIntFunction;

import org.junit.Test;

import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;

import com.openhtmltopdf.extend.FSTextBreaker;
import static com.openhtmltopdf.layout.BreakerTestSupport.*;

public class BreakerTest {
private static class SimpleCharBreaker implements FSTextBreaker {
private String text;
private int pos;

@Override
public int next() {
return pos > text.length() ? -1 : pos++;
}

@Override
public void setText(String newText) {
this.text = newText;
this.pos = 0;
}
}

private static class SimpleLineBreaker implements FSTextBreaker {
private String text;
private int position;

@Override
public int next() {
int ret = text.indexOf(' ', this.position);
this.position = ret < 0 ? -1 : ret + 1;
return ret;
}

@Override
public void setText(String newText) {
this.text = newText;
this.position = 0;
}
}

private FSTextBreaker createLine(String line) {
SimpleLineBreaker breaker = new SimpleLineBreaker();
breaker.setText(line);
return breaker;
}

private FSTextBreaker createChar(String line) {
FSTextBreaker breaker = new SimpleCharBreaker();
breaker.setText(line);
return breaker;
}

private final ToIntFunction<String> MEASURER = (str) -> str.length();
private final ToIntFunction<String> MEASURER3 = (str) -> str.length() * 3;

private LineBreakContext createContext(String str) {
LineBreakContext ctx = new LineBreakContext();
ctx.setMaster(str);
return ctx;
}

@Test
public void testCharacterBreakerSingleChar() {
String whole = "A";
Expand Down Expand Up @@ -108,8 +51,8 @@ public void testCharacterBreakerUntilWord() {

assertFalse(context.isUnbreakable());
assertFalse(context.isNeedsNewLine());
assertThat(context.getWidth(), equalTo(4));
assertThat(context.getEnd(), equalTo(4));
assertThat(context.getWidth(), equalTo(5));
assertThat(context.getEnd(), equalTo(5));
}

@Test
Expand Down
Loading

0 comments on commit 9ab4c10

Please sign in to comment.