From 541545e20a7e85ab730142d2ef9a01185ff032f6 Mon Sep 17 00:00:00 2001 From: danfickle Date: Thu, 10 Jun 2021 17:41:58 +1000 Subject: [PATCH] #364 Failing test for blocks in footnotes. Plus: + Overhauled BlockBoxing while trying to fix issue. Now better performance and cleaner code. + Added benchmarks for BlockBoxing. --- .../com/openhtmltopdf/layout/BlockBoxing.java | 319 +++++++++--------- .../openhtmltopdf/layout/InlineBoxing.java | 7 +- .../openhtmltopdf/layout/LayoutContext.java | 75 ++-- .../com/openhtmltopdf/layout/LayoutState.java | 130 ++++--- .../openhtmltopdf/layout/StyleTracker.java | 65 ++-- .../com/openhtmltopdf/render/BlockBox.java | 12 +- .../benchmark/RenderTextBenchmark.java | 41 ++- .../performance/PerformanceCaseGenerator.java | 60 +++- .../html/issue-364-footnotes-blocks.html | 70 ++++ .../VisualRegressionTest.java | 10 +- 10 files changed, 517 insertions(+), 272 deletions(-) create mode 100644 openhtmltopdf-examples/src/main/resources/visualtest/html/issue-364-footnotes-blocks.html diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockBoxing.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockBoxing.java index 6a3f93fc4..b69e863fd 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockBoxing.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockBoxing.java @@ -21,10 +21,8 @@ package com.openhtmltopdf.layout; import java.awt.Dimension; -import java.util.ArrayList; -import java.util.Iterator; import java.util.List; -import java.util.RandomAccess; +import java.util.TreeSet; import com.openhtmltopdf.css.constants.CSSName; import com.openhtmltopdf.css.constants.IdentValue; @@ -54,34 +52,33 @@ private BlockBoxing() { * {@link ContentType#BLOCK}. */ public static void layoutContent(LayoutContext c, BlockBox block, int contentStart) { - int offset = -1; - List localChildren = block.getChildren(); - if (c.isPrint() && ! (localChildren instanceof RandomAccess)) { - localChildren = new ArrayList<>(localChildren); - } + int size = localChildren.size(); int childOffset = block.getHeight() + contentStart; - RelayoutDataList relayoutDataList = null; + AbstractRelayoutDataList relayoutDataList = null; + if (c.isPrint()) { - relayoutDataList = new RelayoutDataList(localChildren.size()); + relayoutDataList = new LiteRelayoutDataList(size); } int pageCount = NO_PAGE_TRIM; + BlockBox previousChildBox = null; - for (Iterator i = localChildren.iterator(); i.hasNext();) { - BlockBox child = (BlockBox) i.next(); - offset++; + for (int offset = 0; offset < size; offset++) { + BlockBox child = (BlockBox) localChildren.get(offset); - RelayoutData relayoutData = null; + LayoutState savedChildLayoutState = null; boolean mayCheckKeepTogether = false; if (c.isPrint()) { - relayoutData = relayoutDataList.get(offset); - relayoutData.setLayoutState(c.copyStateForRelayout()); - relayoutData.setChildOffset(childOffset); + savedChildLayoutState = c.copyStateForRelayout(); + + relayoutDataList.setLayoutState(offset, savedChildLayoutState); + relayoutDataList.setChildOffset(offset, childOffset); + pageCount = c.getRootLayer().getPages().size(); child.setNeedPageClear(false); @@ -94,28 +91,29 @@ public static void layoutContent(LayoutContext c, BlockBox block, int contentSta } layoutBlockChild( - c, block, child, false, childOffset, NO_PAGE_TRIM, - relayoutData == null ? null : relayoutData.getLayoutState()); + c, block, child, false, childOffset, NO_PAGE_TRIM, savedChildLayoutState); if (c.isPrint()) { boolean needPageClear = child.isNeedPageClear(); if (needPageClear || mayCheckKeepTogether) { c.setMayCheckKeepTogether(mayCheckKeepTogether); + boolean tryToAvoidPageBreak = child.getStyle().isAvoidPageBreakInside() && child.crossesPageBreak(c); boolean keepWithInline = child.isNeedsKeepWithInline(c); if (tryToAvoidPageBreak || needPageClear || keepWithInline) { - c.restoreStateForRelayout(relayoutData.getLayoutState()); + c.restoreStateForRelayout(savedChildLayoutState); + child.reset(c); layoutBlockChild( - c, block, child, true, childOffset, pageCount, relayoutData.getLayoutState()); + c, block, child, true, childOffset, pageCount, savedChildLayoutState); if (tryToAvoidPageBreak && child.crossesPageBreak(c) && ! keepWithInline) { - c.restoreStateForRelayout(relayoutData.getLayoutState()); + c.restoreStateForRelayout(savedChildLayoutState); child.reset(c); layoutBlockChild( - c, block, child, false, childOffset, pageCount, relayoutData.getLayoutState()); + c, block, child, false, childOffset, pageCount, savedChildLayoutState); } } } @@ -143,14 +141,15 @@ public static void layoutContent(LayoutContext c, BlockBox block, int contentSta } if (previousChildBox != null) { - relayoutDataList.markRun(offset, previousChildBox, child); + relayoutDataList.configureRun(offset, previousChildBox, child); } - RelayoutRunResult runResult = - processPageBreakAvoidRun( - c, block, localChildren, offset, relayoutDataList, relayoutData, child); - if (runResult.isChanged()) { - childOffset = runResult.getChildOffset(); + Integer newChildOffset = + processPageBreakAvoidRun( + c, block, localChildren, offset, relayoutDataList, child); + + if (newChildOffset != null) { + childOffset = newChildOffset; if (childOffset > block.getHeight()) { block.setHeight(childOffset); } @@ -161,56 +160,73 @@ public static void layoutContent(LayoutContext c, BlockBox block, int contentSta } } - private static RelayoutRunResult processPageBreakAvoidRun(final LayoutContext c, final BlockBox block, - List localChildren, int offset, - RelayoutDataList relayoutDataList, RelayoutData relayoutData, - BlockBox childBox) { - RelayoutRunResult result = new RelayoutRunResult(); + private static Integer processPageBreakAvoidRun( + LayoutContext c, + BlockBox block, + List localChildren, + int offset, + AbstractRelayoutDataList relayoutDataList, + BlockBox childBox) { + if (offset > 0) { boolean mightNeedRelayout = false; int runEnd = -1; - if (offset == localChildren.size() - 1 && relayoutData.isEndsRun()) { + + if (offset == localChildren.size() - 1 && relayoutDataList.isEndsRun(offset)) { mightNeedRelayout = true; runEnd = offset; } else if (offset > 0) { - RelayoutData previousRelayoutData = relayoutDataList.get(offset - 1); - if (previousRelayoutData.isEndsRun()) { + if (relayoutDataList.isEndsRun(offset - 1)) { mightNeedRelayout = true; runEnd = offset - 1; } } + if (mightNeedRelayout) { int runStart = relayoutDataList.getRunStart(runEnd); + int newChildOffset; + if ( isPageBreakBetweenChildBoxes(relayoutDataList, runStart, runEnd, c, block) ) { - result.setChanged(true); block.resetChildren(c, runStart, offset); - result.setChildOffset(relayoutRun(c, localChildren, block, - relayoutDataList, runStart, offset, true)); + + newChildOffset = relayoutRun( + c, localChildren, block, + relayoutDataList, runStart, offset, true); + if ( isPageBreakBetweenChildBoxes(relayoutDataList, runStart, runEnd, c, block) ) { block.resetChildren(c, runStart, offset); - result.setChildOffset(relayoutRun(c, localChildren, block, - relayoutDataList, runStart, offset, false)); + newChildOffset = relayoutRun( + c, localChildren, block, + relayoutDataList, runStart, offset, false); } + + return Integer.valueOf(newChildOffset); } } } - return result; + + return null; } - private static boolean isPageBreakBetweenChildBoxes(RelayoutDataList relayoutDataList, + private static boolean isPageBreakBetweenChildBoxes( + AbstractRelayoutDataList relayoutDataList, int runStart, int runEnd, LayoutContext c, BlockBox block) { + for ( int i = runStart; i < runEnd; i++ ) { Box prevChild = block.getChild(i); Box nextChild = block.getChild(i+1); + // if nextChild is made of several lines, then only the first line // is relevant for "page-break-before: avoid". Box nextLine = getFirstLine(nextChild) == null ? nextChild : getFirstLine(nextChild); int prevChildEnd = prevChild.getAbsY() + prevChild.getHeight(); int nextLineEnd = nextLine.getAbsY() + nextLine.getHeight(); + if ( c.getRootLayer().crossesPageBreak(c, prevChildEnd, nextLineEnd) ) { return true; } } + return false; } @@ -225,8 +241,8 @@ private static LineBox getFirstLine(Box box) { private static int relayoutRun( LayoutContext c, List localChildren, BlockBox block, - RelayoutDataList relayoutDataList, int start, int end, boolean onNewPage) { - int childOffset = relayoutDataList.get(start).getChildOffset(); + AbstractRelayoutDataList relayoutDataList, int start, int end, boolean onNewPage) { + int childOffset = relayoutDataList.getChildOffset(start); if (onNewPage) { Box startBox = localChildren.get(start); @@ -237,44 +253,47 @@ private static int relayoutRun( // reset height of parent as it is used for Y-setting of children block.setHeight(childOffset); - for (int i = start; i <= end; i++) { BlockBox child = (BlockBox) localChildren.get(i); - - RelayoutData relayoutData = relayoutDataList.get(i); - int pageCount = c.getRootLayer().getPages().size(); - //TODO:handle run-ins. For now, treat them as blocks + LayoutState restoredChildLayoutState = relayoutDataList.getLayoutState(i); + c.restoreStateForRelayout(restoredChildLayoutState); - c.restoreStateForRelayout(relayoutData.getLayoutState()); - relayoutData.setChildOffset(childOffset); + relayoutDataList.setChildOffset(i, childOffset); boolean mayCheckKeepTogether = false; + if ((child.getStyle().isAvoidPageBreakInside() || child.getStyle().isKeepWithInline()) && c.isMayCheckKeepTogether()) { mayCheckKeepTogether = true; c.setMayCheckKeepTogether(false); } + layoutBlockChild( - c, block, child, false, childOffset, NO_PAGE_TRIM, relayoutData.getLayoutState()); + c, block, child, false, childOffset, NO_PAGE_TRIM, restoredChildLayoutState); if (mayCheckKeepTogether) { c.setMayCheckKeepTogether(true); + boolean tryToAvoidPageBreak = child.getStyle().isAvoidPageBreakInside() && child.crossesPageBreak(c); + boolean needPageClear = child.isNeedPageClear(); boolean keepWithInline = child.isNeedsKeepWithInline(c); + if (tryToAvoidPageBreak || needPageClear || keepWithInline) { - c.restoreStateForRelayout(relayoutData.getLayoutState()); + c.restoreStateForRelayout(restoredChildLayoutState); child.reset(c); + layoutBlockChild( - c, block, child, true, childOffset, pageCount, relayoutData.getLayoutState()); + c, block, child, true, childOffset, pageCount, restoredChildLayoutState); if (tryToAvoidPageBreak && child.crossesPageBreak(c) && ! keepWithInline) { - c.restoreStateForRelayout(relayoutData.getLayoutState()); + c.restoreStateForRelayout(restoredChildLayoutState); child.reset(c); + layoutBlockChild( - c, block, child, false, childOffset, pageCount, relayoutData.getLayoutState()); + c, block, child, false, childOffset, pageCount, restoredChildLayoutState); } } } @@ -282,6 +301,7 @@ private static int relayoutRun( c.getRootLayer().ensureHasPage(c, child); Dimension relativeOffset = child.getRelativeOffset(); + if (relativeOffset == null) { childOffset = child.getY() + child.getHeight(); } else { @@ -362,132 +382,125 @@ private static void repositionBox(LayoutContext c, BlockBox child, int trimmedPa } } - private static class RelayoutDataList { - private List _hints; - - public RelayoutDataList(int size) { - _hints = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - _hints.add(new RelayoutData()); - } - } + /** + * If we should try to avoid a page break between two block boxes. + */ + public static boolean avoidPageBreakBetween(BlockBox previous, BlockBox current) { + IdentValue previousAfter = + previous.getStyle().getIdent(CSSName.PAGE_BREAK_AFTER); + IdentValue currentBefore = + current.getStyle().getIdent(CSSName.PAGE_BREAK_BEFORE); + + return (previousAfter == IdentValue.AVOID && currentBefore == IdentValue.AUTO) || + (previousAfter == IdentValue.AUTO && currentBefore == IdentValue.AVOID) || + (previousAfter == IdentValue.AVOID && currentBefore == IdentValue.AVOID); + } - public RelayoutData get(int index) { - return _hints.get(index); - } + private abstract static class AbstractRelayoutDataList { + abstract int getChildOffset(int boxIndex); + abstract LayoutState getLayoutState(int boxIndex); - public void markRun(int offset, BlockBox previous, BlockBox current) { - RelayoutData previousData = get(offset - 1); - RelayoutData currentData = get(offset); + abstract void setLayoutState(int boxIndex, LayoutState state); + abstract void setChildOffset(int boxIndex, int childOffset); - IdentValue previousAfter = - previous.getStyle().getIdent(CSSName.PAGE_BREAK_AFTER); - IdentValue currentBefore = - current.getStyle().getIdent(CSSName.PAGE_BREAK_BEFORE); + abstract int getRunStart(int endRunIndex); - if ((previousAfter == IdentValue.AVOID && currentBefore == IdentValue.AUTO) || - (previousAfter == IdentValue.AUTO && currentBefore == IdentValue.AVOID) || - (previousAfter == IdentValue.AVOID && currentBefore == IdentValue.AVOID)) { - if (! previousData.isInRun()) { - previousData.setStartsRun(true); - } - previousData.setInRun(true); - currentData.setInRun(true); + abstract boolean isEndsRun(int boxIndex); - if (offset == _hints.size() - 1) { - currentData.setEndsRun(true); - } - } else { - if (previousData.isInRun()) { - previousData.setEndsRun(true); - } - } - } - - public int getRunStart(int runEnd) { - int offset = runEnd; - RelayoutData current = get(offset); - if (! current.isEndsRun()) { - throw new RuntimeException("Not the end of a run"); - } - while (! current.isStartsRun()) { - current = get(--offset); - } - return offset; - } + abstract void configureRun(int boxIndex, BlockBox previous, BlockBox current); } - private static class RelayoutRunResult { - private boolean _changed; - private int _childOffset; + private static class LiteRelayoutDataList extends AbstractRelayoutDataList { + final int[] childOffsets; + final LayoutState[] layoutStates; + TreeSet runStarts; + TreeSet runEnds; - public boolean isChanged() { - return _changed; + LiteRelayoutDataList(int size) { + childOffsets = new int[size]; + layoutStates = new LayoutState[size]; } - public void setChanged(boolean changed) { - _changed = changed; + @Override + int getChildOffset(int boxIndex) { + return childOffsets[boxIndex]; } - public int getChildOffset() { - return _childOffset; + @Override + LayoutState getLayoutState(int boxIndex) { + return layoutStates[boxIndex]; } - public void setChildOffset(int childOffset) { - _childOffset = childOffset; + @Override + void setLayoutState(int boxIndex, LayoutState state) { + layoutStates[boxIndex] = state; } - } - private static class RelayoutData { - private LayoutState _layoutState; - - private boolean _startsRun; - private boolean _endsRun; - private boolean _inRun; - - private int _childOffset; - - public RelayoutData() { - } - - public boolean isEndsRun() { - return _endsRun; + @Override + void setChildOffset(int boxIndex, int childOffset) { + childOffsets[boxIndex] = childOffset; } - public void setEndsRun(boolean endsRun) { - _endsRun = endsRun; + @Override + boolean isEndsRun(int boxIndex) { + return runEnds != null && runEnds.contains(boxIndex); } - public boolean isInRun() { - return _inRun; + @Override + int getRunStart(int endRunIndex) { + return runStarts.floor(endRunIndex); } - public void setInRun(boolean inRun) { - _inRun = inRun; - } + boolean isInRun(int boxIndex) { + if (runStarts == null) { + return false; + } - public LayoutState getLayoutState() { - return _layoutState; - } + Integer lastRunStart = runStarts.floor(boxIndex); + if (lastRunStart != null) { + Integer lastRunEnd = runEnds != null ? runEnds.floor(boxIndex) : null; + return (lastRunEnd == null || lastRunEnd >= boxIndex); + } - public void setLayoutState(LayoutState layoutState) { - _layoutState = layoutState; + return false; } - public boolean isStartsRun() { - return _startsRun; + void addRunStart(int boxIndex) { + if (runStarts == null) { + runStarts = new TreeSet<>(); + } + runStarts.add(boxIndex); } - public void setStartsRun(boolean startsRun) { - _startsRun = startsRun; + void addRunEnd(int boxIndex) { + if (runEnds == null) { + runEnds = new TreeSet<>(); + } + runEnds.add(boxIndex); } - public int getChildOffset() { - return _childOffset; - } + /** + * Marks two consecutive block boxes as being in a run of boxes where + * a page break should not occur between them as set in the + * page-break-after and page-break-before + * CSS properties. + */ + @Override + public void configureRun(int offset, BlockBox previous, BlockBox current) { + boolean previousInRun = isInRun(offset - 1); + + if (avoidPageBreakBetween(previous, current)) { + if (!previousInRun) { + addRunStart(offset - 1); + } - public void setChildOffset(int childOffset) { - _childOffset = childOffset; + if (offset == childOffsets.length) { + addRunEnd(offset); + } + } else if (previousInRun) { + addRunEnd(offset - 1); + } } } + } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java index 1d8628d4d..70777abe1 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java @@ -580,7 +580,9 @@ private static InlineLayoutBox addFirstLetterBox(LayoutContext c, LineBox curren lbContext.setStart(lbContext.getEnd()); - c.getFirstLettersTracker().clearStyles(); + c.setFirstLettersTracker( + StyleTracker.withNoStyles()); + currentIB.setStyle(previous); return iB; @@ -1066,7 +1068,8 @@ private static void saveLine(LineBox current, LayoutContext c, } if (hasFirstLinePCs && current.isFirstLine()) { - c.getFirstLinesTracker().clearStyles(); + c.setFirstLinesTracker( + StyleTracker.withNoStyles()); block.styleText(c); } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutContext.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutContext.java index 220f8d61f..f714b66e6 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutContext.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutContext.java @@ -94,6 +94,8 @@ public class LayoutContext implements CssContext { private boolean _isInFloatBottom; + private LayoutState _savedLayoutState; + private int _footnoteIndex; private FootnoteManager _footnoteManager; @@ -175,13 +177,13 @@ public void setDefaultTextDirection(byte direction) { _bfcs = new LinkedList<>(); _layers = new LinkedList<>(); - _firstLines = new StyleTracker(); - _firstLetters = new StyleTracker(); + _firstLines = StyleTracker.withNoStyles(); + _firstLetters = StyleTracker.withNoStyles(); } public void reInit(boolean keepLayers) { - _firstLines = new StyleTracker(); - _firstLetters = new StyleTracker(); + _firstLines = StyleTracker.withNoStyles(); + _firstLetters = StyleTracker.withNoStyles(); _currentMarkerData = null; _bfcs = new LinkedList<>(); @@ -196,22 +198,23 @@ public void reInit(boolean keepLayers) { } public LayoutState captureLayoutState() { - LayoutState result = new LayoutState(); - - result.setFirstLines(_firstLines); - result.setFirstLetters(_firstLetters); - result.setCurrentMarkerData(_currentMarkerData); - - result.setBFCs(_bfcs); - - if (isPrint()) { - result.setPageName(getPageName()); - result.setExtraSpaceBottom(getExtraSpaceBottom()); - result.setExtraSpaceTop(getExtraSpaceTop()); - result.setNoPageBreak(getNoPageBreak()); + if (!isPrint()) { + return new LayoutState( + _bfcs, + _currentMarkerData, + _firstLetters, + _firstLines); + } else { + return new LayoutState( + _bfcs, + _currentMarkerData, + _firstLetters, + _firstLines, + getPageName(), + getExtraSpaceTop(), + getExtraSpaceBottom(), + getNoPageBreak()); } - - return result; } public void restoreLayoutState(LayoutState layoutState) { @@ -231,17 +234,24 @@ public void restoreLayoutState(LayoutState layoutState) { } public LayoutState copyStateForRelayout() { - LayoutState result = new LayoutState(); - - result.setFirstLetters(_firstLetters.copyOf()); - result.setFirstLines(_firstLines.copyOf()); - result.setCurrentMarkerData(_currentMarkerData); - - if (isPrint()) { - result.setPageName(getPageName()); + if (_savedLayoutState != null && + _savedLayoutState.equal( + _currentMarkerData, + _firstLetters, + _firstLines, + isPrint() ? getPageName() : null)) { + + return _savedLayoutState; } - return result; + _savedLayoutState = + new LayoutState( + _firstLetters, + _firstLines, + _currentMarkerData, + isPrint() ? getPageName() : null); + + return _savedLayoutState; } public void restoreStateForRelayout(LayoutState layoutState) { @@ -559,4 +569,13 @@ public FootnoteManager getFootnoteManager() { return _footnoteManager; } + + public void setFirstLettersTracker(StyleTracker firstLetters) { + _firstLetters = firstLetters; + } + + public void setFirstLinesTracker(StyleTracker firstLines) { + _firstLines = firstLines; + } + } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutState.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutState.java index e500ecc04..29863db4f 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutState.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LayoutState.java @@ -20,89 +20,129 @@ package com.openhtmltopdf.layout; import java.util.LinkedList; +import java.util.Objects; import com.openhtmltopdf.render.MarkerData; /** - * A bean which captures all state necessary to lay out an arbitrary box. + * A bean which captures all state necessary to lay out an arbitrary box. * Mutable objects must be copied when provided to this class. It is far too - * expensive to maintain a bean of this class for each box. + * expensive to maintain a bean of this class for each box. * It is only created as needed. + *

+ * IMPORTANT: Immutable after construction. */ public class LayoutState { - private StyleTracker _firstLines; - private StyleTracker _firstLetters; - - private MarkerData _currentMarkerData; - - private LinkedList _BFCs; - - private String _pageName; - private int _extraSpaceTop; - private int _extraSpaceBottom; - private int _noPageBreak; - - public LinkedList getBFCs() { - return _BFCs; + private final StyleTracker _firstLines; + private final StyleTracker _firstLetters; + + private final MarkerData _currentMarkerData; + + private final LinkedList _BFCs; + + private final String _pageName; + private final int _extraSpaceTop; + private final int _extraSpaceBottom; + private final int _noPageBreak; + + public LayoutState( + LinkedList bfcs, + MarkerData markerData, + StyleTracker firstLetters, + StyleTracker firstLines, + String pageName, + int extraSpaceTop, + int extraSpaceBottom, + int noPageBreak) { + this._BFCs = bfcs; + this._currentMarkerData = markerData; + this._firstLetters = firstLetters; + this._firstLines = firstLines; + this._pageName = pageName; + this._extraSpaceTop = extraSpaceTop; + this._extraSpaceBottom = extraSpaceBottom; + this._noPageBreak = noPageBreak; + } + + public LayoutState( + LinkedList bfcs, + MarkerData currentMarkerData, + StyleTracker firstLetters, + StyleTracker firstLines) { + this(bfcs, currentMarkerData, firstLetters, firstLines, null, 0, 0, 0); + } + + public LayoutState( + StyleTracker firstLetters, + StyleTracker firstLines, + MarkerData currentMarkerData, + String pageName) { + this(null, currentMarkerData, firstLetters, firstLines, pageName, 0, 0, 0); + } + + public boolean equal( + LinkedList bfcs, + MarkerData markerData, + StyleTracker firstLetters, + StyleTracker firstLines, + String pageName, + int extraSpaceTop, + int extraSpaceBottom, + int noPageBreak) { + + return bfcs == _BFCs && + markerData == _currentMarkerData && + Objects.equals(firstLetters, _firstLetters) && + Objects.equals(firstLines, _firstLines) && + Objects.equals(pageName, _pageName) && + extraSpaceTop == _extraSpaceTop && + extraSpaceBottom == _extraSpaceBottom && + noPageBreak == _noPageBreak; + } + + public boolean equal( + MarkerData currentMarkerData, + StyleTracker firstLetters, + StyleTracker firstLines, + String pageName) { + + return currentMarkerData == _currentMarkerData && + Objects.equals(firstLetters, _firstLetters) && + Objects.equals(firstLines, _firstLines) && + Objects.equals(pageName, _pageName); } - public void setBFCs(LinkedList s) { - _BFCs = s; + + public LinkedList getBFCs() { + return _BFCs; } public MarkerData getCurrentMarkerData() { return _currentMarkerData; } - public void setCurrentMarkerData(MarkerData currentMarkerData) { - _currentMarkerData = currentMarkerData; - } - public StyleTracker getFirstLetters() { return _firstLetters; } - public void setFirstLetters(StyleTracker firstLetters) { - _firstLetters = firstLetters; - } - public StyleTracker getFirstLines() { return _firstLines; } - public void setFirstLines(StyleTracker firstLines) { - _firstLines = firstLines; - } - public String getPageName() { return _pageName; } - public void setPageName(String pageName) { - _pageName = pageName; - } - public int getExtraSpaceTop() { return _extraSpaceTop; } - public void setExtraSpaceTop(int extraSpaceTop) { - _extraSpaceTop = extraSpaceTop; - } - public int getExtraSpaceBottom() { return _extraSpaceBottom; } - public void setExtraSpaceBottom(int extraSpaceBottom) { - _extraSpaceBottom = extraSpaceBottom; - } - public int getNoPageBreak() { return _noPageBreak; } - public void setNoPageBreak(int noPageBreak) { - _noPageBreak = noPageBreak; - } } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/StyleTracker.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/StyleTracker.java index 65722d050..f4d64d3b8 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/StyleTracker.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/StyleTracker.java @@ -20,7 +20,6 @@ package com.openhtmltopdf.layout; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import com.openhtmltopdf.css.newmatch.CascadedStyle; @@ -30,43 +29,67 @@ * A managed list of {@link CalculatedStyle} objects. It is used when keeping * track of the styles which apply to a :first-line or :first-letter pseudo * element. + *

+ * IMPORTANT: Immutable after constructor. */ public class StyleTracker { - private List _styles = new ArrayList<>(); - - public void addStyle(CascadedStyle style) { - _styles.add(style); + private final List _styles; + + private static final StyleTracker EMPTY_INSTANCE = new StyleTracker(0); + + public StyleTracker(int size) { + this._styles = new ArrayList<>(size); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + + if (obj.getClass() != this.getClass()) { + return false; + } + + return ((StyleTracker) obj).getStyles().equals(this.getStyles()); + } + + public StyleTracker withStyle(CascadedStyle style) { + StyleTracker tracker = new StyleTracker(getStyles().size() + 1); + tracker._styles.addAll(getStyles()); + tracker._styles.add(style); + return tracker; } - public void removeLast() { - if (_styles.size() != 0) { - _styles.remove(_styles.size()-1); + public StyleTracker withOutLast() { + if (_styles.isEmpty()) { + return this; + } else if (_styles.size() == 1) { + return EMPTY_INSTANCE; } + + StyleTracker tracker = new StyleTracker(getStyles().size() - 1); + tracker._styles.addAll(getStyles().subList(0, getStyles().size() - 1)); + return tracker; } public boolean hasStyles() { - return _styles.size() != 0; + return !_styles.isEmpty(); } - public void clearStyles() { - _styles.clear(); + public static StyleTracker withNoStyles() { + return EMPTY_INSTANCE; } - + public CalculatedStyle deriveAll(CalculatedStyle start) { CalculatedStyle result = start; - for (Iterator i = getStyles().iterator(); i.hasNext(); ) { - result = result.deriveStyle(i.next()); + for (CascadedStyle style : getStyles()) { + result = result.deriveStyle(style); } return result; } - public List getStyles() { + private List getStyles() { return _styles; } - - public StyleTracker copyOf() { - StyleTracker result = new StyleTracker(); - result._styles.addAll(this._styles); - return result; - } } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/BlockBox.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/BlockBox.java index 5ca3e2fc4..50da727b7 100755 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/BlockBox.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/BlockBox.java @@ -1247,10 +1247,12 @@ protected void layoutChildren(LayoutContext c, int contentStart) { ensureChildren(c); if (getFirstLetterStyle() != null) { - c.getFirstLettersTracker().addStyle(getFirstLetterStyle()); + c.setFirstLettersTracker( + c.getFirstLettersTracker().withStyle(getFirstLetterStyle())); } if (getFirstLineStyle() != null) { - c.getFirstLinesTracker().addStyle(getFirstLineStyle()); + c.setFirstLinesTracker( + c.getFirstLinesTracker().withStyle(getFirstLineStyle())); } switch (getChildrenContentType()) { @@ -1269,10 +1271,12 @@ protected void layoutChildren(LayoutContext c, int contentStart) { } if (getFirstLetterStyle() != null) { - c.getFirstLettersTracker().removeLast(); + c.setFirstLettersTracker( + c.getFirstLettersTracker().withOutLast()); } if (getFirstLineStyle() != null) { - c.getFirstLinesTracker().removeLast(); + c.setFirstLinesTracker( + c.getFirstLinesTracker().withOutLast()); } setState(Box.DONE); diff --git a/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/benchmark/RenderTextBenchmark.java b/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/benchmark/RenderTextBenchmark.java index 16b0107dc..18745dde9 100644 --- a/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/benchmark/RenderTextBenchmark.java +++ b/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/benchmark/RenderTextBenchmark.java @@ -1,15 +1,19 @@ package com.openhtmltopdf.benchmark; import com.openhtmltopdf.pdfboxout.PdfRendererBuilder; +import com.openhtmltopdf.performance.PerformanceCaseGenerator; import com.openhtmltopdf.util.XRLog; import org.apache.pdfbox.io.IOUtils; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.runner.Runner; import org.openjdk.jmh.runner.options.Options; import org.openjdk.jmh.runner.options.OptionsBuilder; @@ -24,17 +28,27 @@ import java.util.concurrent.TimeUnit; /** + * To run these benchmarks in the repo root directory: + * 
+ * mvn install -DskipTests
+ * java -jar ./openhtmltopdf-examples/target/benchmarks.jar
+ *
+ * + * They should take a couple of minutes. + * * @author schrader */ @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.MILLISECONDS) @State(Scope.Thread) +@Warmup(iterations = 2, time = 3, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 2, time = 6, timeUnit = TimeUnit.SECONDS) +@Fork(warmups = 0, value = 1) public class RenderTextBenchmark { public static void main(String[] args) throws Exception { Options opt = new OptionsBuilder() .include(RenderTextBenchmark.class.getSimpleName()) - .forks(1) .build(); new Runner(opt).run(); @@ -50,6 +64,11 @@ public void setUp() { "/benchmark/render-text-plain.html", "/benchmark/render-text-soft-hyphens.html" ).forEach(path -> contents.put(path, readContent(path))); + + contents.put("/performance/table-rows", PerformanceCaseGenerator.tableRows(1_000)); + contents.put("/performance/paragraphs", PerformanceCaseGenerator.paragraphs(100)); + contents.put("/performance/page-break-blocks", PerformanceCaseGenerator.pageBreakAvoidBlocks(300)); + contents.put("/performance/blocks", PerformanceCaseGenerator.blocks(300)); } @Benchmark @@ -62,6 +81,26 @@ public void renderText_SoftHyphens() throws Exception { runRenderer(contents.get("/benchmark/render-text-soft-hyphens.html")); } + @Benchmark + public void renderTableRows() throws IOException { + runRenderer(contents.get("/performance/table-rows")); + } + + @Benchmark + public void renderParagraphs() throws IOException { + runRenderer(contents.get("/performance/paragraphs")); + } + + @Benchmark + public void renderBlocksPageBreak() throws IOException { + runRenderer(contents.get("/performance/page-break-blocks")); + } + + @Benchmark + public void renderBlocks() throws IOException { + runRenderer(contents.get("/performance/blocks")); + } + private void runRenderer(String html) throws IOException { ByteArrayOutputStream actual = new ByteArrayOutputStream(); diff --git a/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/performance/PerformanceCaseGenerator.java b/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/performance/PerformanceCaseGenerator.java index ecf4bf237..ea84aa015 100644 --- a/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/performance/PerformanceCaseGenerator.java +++ b/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/performance/PerformanceCaseGenerator.java @@ -3,6 +3,8 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import com.openhtmltopdf.layout.BlockBoxing; + public class PerformanceCaseGenerator { private static final String LOREM = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. " + @@ -10,29 +12,57 @@ public class PerformanceCaseGenerator { "eget bibendum nulla massa vel metus. Duis sed nunc ornare, convallis purus at, " + "fringilla ipsum. Quisque ullamcorper hendrerit ipsum at eleifend. Orci varius " + "natoque penatibus et magnis dis parturient montes, nascetur ridiculus."; - - + + private static String join(String hdr, String repeat, String ftr, int howMany) { + return IntStream.range(0, howMany) + .mapToObj(i -> repeat) + .collect(Collectors.joining("\n", hdr, ftr)); + } + public static String paragraphs(int howMany) { final String hdr = ""; final String paragraph = "

" + LOREM + "

"; final String ftr = ""; - - return IntStream.range(0, howMany) - .mapToObj(i -> paragraph) - .collect(Collectors.joining("\n", hdr, ftr)); + + return join(hdr, paragraph, ftr, howMany); } - + public static String tableRows(int howMany) { final String hdr = ""; final String tr = ""; final String ftr = "
OneTwoThree
"; - - return IntStream.range(0, howMany) - .mapToObj(i -> tr) - .collect(Collectors.joining("\n", hdr, ftr)); + + return join(hdr, tr, ftr, howMany); } - + + /** + * Performance of {@link BlockBoxing} + */ + public static String pageBreakAvoidBlocks(int howMany) { + final String hdr = ""; + final String block = "
"; + final String ftr = ""; + + return join(hdr, block, ftr, howMany); + } + + /** + * Performance of {@link BlockBoxing} + */ + public static String blocks(int howMany) { + final String hdr = ""; + final String block = "
"; + final String ftr = ""; + + return join(hdr, block, ftr, howMany); + } + /** * Performace case for: * Issue 396 - CSS border-radius makes pdf rendering very slow. @@ -47,10 +77,8 @@ public static String borderRadius(int howMany) { ""; final String div = "
"; final String ftr = ""; - - return IntStream.range(0, howMany) - .mapToObj(i -> div) - .collect(Collectors.joining("\n", hdr, ftr)); + + return join(hdr, div, ftr, howMany); } } diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-364-footnotes-blocks.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-364-footnotes-blocks.html new file mode 100644 index 000000000..29a99a9b9 --- /dev/null +++ b/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-364-footnotes-blocks.html @@ -0,0 +1,70 @@ + + + + + +
+
+ +
+

+ Footnote with large text! +

+

+ Another paragraph! +

+
+
+ This however is a purely inline footnote. +
+ +
+
+
+ + diff --git a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java index 6b0d3539a..bf5b70a86 100644 --- a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java +++ b/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java @@ -1417,7 +1417,7 @@ public void testIssue649MultipleBgImagesPageBox() throws IOException { * Tests that we support CSS footnotes. */ @Test - @Ignore // Footnotes not implemented. + @Ignore // Working well, final touches. public void testIssue364FootnotesBasicExample() throws IOException { assertTrue(vt.runTest("issue-364-footnotes-basic")); } @@ -1426,7 +1426,7 @@ public void testIssue364FootnotesBasicExample() throws IOException { * Tests that a line of text can contain two multi-page footnotes. */ @Test - @Ignore // Not perfect yet. + @Ignore // Final touches. public void testIssue364FootnotesMultiPage() throws IOException { assertTrue(vt.runTest("issue-364-footnotes-multi-page")); } @@ -1441,6 +1441,12 @@ public void testIssue364FootnotesTooLarge() throws IOException { assertTrue(vt.runTest("issue-364-footnotes-too-large")); } + @Test + @Ignore // Failing badly. + public void testIssue364FootnotesBlocks() throws IOException { + assertTrue(vt.runTest("issue-364-footnotes-blocks")); + } + // TODO: // + Elements that appear just on generated overflow pages. // + content property (page counters, etc)