You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Vincent Hennebert <vi...@anyware-tech.com> on 2008/03/17 20:29:07 UTC
Re: svn commit: r637119 - in /xmlgraphics/fop/branches/fop-0_95:
src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java status.xml
test/layoutengine/standard-testcases/block_break-before_bug44412.xml
> Author: jeremias
> Date: Fri Mar 14 07:41:03 2008
> New Revision: 637119
>
> URL: http://svn.apache.org/viewvc?rev=637119&view=rev
> Log:
> Bugzilla #44412:
> Bugfix: The before border of a block is no longer swallowed if its first child issues a break-before.
>
> Added:
> xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml (with props)
> Modified:
> xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
> xmlgraphics/fop/branches/fop-0_95/status.xml
>
> Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java?rev=637119&r1=637118&r2=637119&view=diff
> ==============================================================================
> --- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java (original)
> +++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java Fri Mar 14 07:41:03 2008
> @@ -294,15 +294,24 @@
> if (returnedList != null
> && returnedList.size() == 1
> && ((ListElement) returnedList.getFirst()).isForcedBreak()) {
> - // a descendant of this block has break-before
> - contentList.addAll(returnedList);
>
> if (curLM.isFinished() && !hasNextChildLM()) {
> + // a descendant of this block has break-before
> + contentList.addAll(returnedList);
> +
> forcedBreakAfterLast = (BreakElement)contentList.removeLast();
> context.clearPendingMarks();
> break;
> }
What’s the point in adding a one-element list to another one if this
element is removed just after that? What have I missed?
Vincent
--
Vincent Hennebert Anyware Technologies
http://people.apache.org/~vhennebert http://www.anyware-tech.com
Apache FOP Committer FOP Development/Consulting
Re: svn commit: r637119 - in /xmlgraphics/fop/branches/fop-0_95:
src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java status.xml
test/layoutengine/standard-testcases/block_break-before_bug44412.xml
Posted by Vincent Hennebert <vi...@anyware-tech.com>.
Jeremias Maerki wrote:
> On 17.03.2008 20:29:07 Vincent Hennebert wrote:
>>> Author: jeremias
>>> Date: Fri Mar 14 07:41:03 2008
>>> New Revision: 637119
<snip/>
>>> if (returnedList != null
>>> && returnedList.size() == 1
>>> && ((ListElement) returnedList.getFirst()).isForcedBreak()) {
>>> - // a descendant of this block has break-before
>>> - contentList.addAll(returnedList);
>>>
>>> if (curLM.isFinished() && !hasNextChildLM()) {
>>> + // a descendant of this block has break-before
>>> + contentList.addAll(returnedList);
>>> +
>>> forcedBreakAfterLast = (BreakElement)contentList.removeLast();
>>> context.clearPendingMarks();
>>> break;
>>> }
>> What’s the point in adding a one-element list to another one if this
>> element is removed just after that? What have I missed?
>
> No particular point. It was refactoring to conserve existing
> functionality without looking at the bigger context. The if statement
> could probably be rewritten like this to "optimize" it.
>
> if (curLM.isFinished() && !hasNextChildLM()) {
> // a descendant of this block has break-before
> forcedBreakAfterLast = (BreakElement)contentList.removeLast();
> assert contentList.size() == 0;
> context.clearPendingMarks();
> break;
> }
Again, what was the additional cost of applying such a change? Isn’t the
code of this method already complex enough?
Vincent
--
Vincent Hennebert Anyware Technologies
http://people.apache.org/~vhennebert http://www.anyware-tech.com
Apache FOP Committer FOP Development/Consulting
Re: svn commit: r637119 - in /xmlgraphics/fop/branches/fop-0_95: src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java status.xml test/layoutengine/standard-testcases/block_break-before_bug44412.xml
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 17.03.2008 20:29:07 Vincent Hennebert wrote:
> > Author: jeremias
> > Date: Fri Mar 14 07:41:03 2008
> > New Revision: 637119
> >
> > URL: http://svn.apache.org/viewvc?rev=637119&view=rev
> > Log:
> > Bugzilla #44412:
> > Bugfix: The before border of a block is no longer swallowed if its first child issues a break-before.
> >
> > Added:
> > xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml (with props)
> > Modified:
> > xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
> > xmlgraphics/fop/branches/fop-0_95/status.xml
> >
> > Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
> > URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java?rev=637119&r1=637118&r2=637119&view=diff
> > ==============================================================================
> > --- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java (original)
> > +++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java Fri Mar 14 07:41:03 2008
> > @@ -294,15 +294,24 @@
> > if (returnedList != null
> > && returnedList.size() == 1
> > && ((ListElement) returnedList.getFirst()).isForcedBreak()) {
> > - // a descendant of this block has break-before
> > - contentList.addAll(returnedList);
> >
> > if (curLM.isFinished() && !hasNextChildLM()) {
> > + // a descendant of this block has break-before
> > + contentList.addAll(returnedList);
> > +
> > forcedBreakAfterLast = (BreakElement)contentList.removeLast();
> > context.clearPendingMarks();
> > break;
> > }
>
> What’s the point in adding a one-element list to another one if this
> element is removed just after that? What have I missed?
No particular point. It was refactoring to conserve existing
functionality without looking at the bigger context. The if statement
could probably be rewritten like this to "optimize" it.
if (curLM.isFinished() && !hasNextChildLM()) {
// a descendant of this block has break-before
forcedBreakAfterLast = (BreakElement)contentList.removeLast();
assert contentList.size() == 0;
context.clearPendingMarks();
break;
}
Jeremias Maerki