You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by ad...@apache.org on 2006/01/09 22:20:15 UTC
svn commit: r367395 -
/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Author: adelmelle
Date: Mon Jan 9 13:20:07 2006
New Revision: 367395
URL: http://svn.apache.org/viewcvs?rev=367395&view=rev
Log:
Slight enhancement to white-space-handling: removal of trailing white-space for trailing inlines
Modified:
xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
URL: http://svn.apache.org/viewcvs/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java?rev=367395&r1=367394&r2=367395&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java Mon Jan 9 13:20:07 2006
@@ -46,6 +46,8 @@
private RecursiveCharIterator charIter;
private List discardableFOCharacters;
+ private List pendingInlines;
+ private CharIterator firstWhiteSpaceInSeq;
/**
* Marks a Character object as discardable, so that it is effectively
@@ -80,19 +82,36 @@
}
charIter = new RecursiveCharIterator(fo, firstTextNode);
inWhiteSpace = false;
- if (fo.getNameId() == Constants.FO_BLOCK) {
- int textNodeIndex = -1;
- if (fo.childNodes != null) {
- textNodeIndex = fo.childNodes.indexOf(firstTextNode);
- }
- afterLinefeed = (textNodeIndex == 0
- || ((FONode) fo.childNodes.get(textNodeIndex - 1))
- .getNameId() == Constants.FO_BLOCK);
+ int textNodeIndex = -1;
+ if (currentFO == currentBlock) {
+ textNodeIndex = fo.childNodes.indexOf(firstTextNode);
+ afterLinefeed = ((textNodeIndex == 0)
+ || (textNodeIndex > 0
+ && ((FONode) fo.childNodes.get(textNodeIndex - 1))
+ .getNameId() == Constants.FO_BLOCK));
+ endOfBlock = (nextChild == null);
}
- endOfBlock = (nextChild == null && currentFO == currentBlock);
nextChildIsBlock = (nextChild != null
- && nextChild.getNameId() == Constants.FO_BLOCK);
+ && nextChild.getNameId() == Constants.FO_BLOCK);
handleWhiteSpace();
+ if (inWhiteSpace) {
+ if (!endOfBlock && currentFO != currentBlock) {
+ /* means there is at least one trailing space in the
+ inline FO that is about to end */
+ addPendingInline(fo);
+ } else if (pendingInlines != null) {
+ if (endOfBlock || nextChildIsBlock) {
+ /* handle white-space for all trailing inlines*/
+ for (int i = pendingInlines.size(); --i >= 0;) {
+ PendingInline p = (PendingInline) pendingInlines.get(i);
+ currentFO = p.fo;
+ charIter = (RecursiveCharIterator) p.firstTrailingWhiteSpace;
+ handleWhiteSpace();
+ }
+ }
+ pendingInlines.clear();
+ }
+ }
}
/**
@@ -159,6 +178,9 @@
if (bIgnore) {
charIter.remove();
} else {
+ if (!inWhiteSpace) {
+ firstWhiteSpaceInSeq = charIter.mark();
+ }
// this is to retain a single space between words
inWhiteSpace = true;
if (currentChar != '\u0020') {
@@ -190,7 +212,7 @@
case CharUtilities.EOT:
// A "boundary" objects such as non-character inline
- // or nested block object was encountered.
+ // or nested block object was encountered. (? can't happen)
// If any whitespace run in progress, finish it.
// FALL THROUGH
@@ -209,12 +231,19 @@
}
}
+ private void addPendingInline(FObjMixed fo) {
+ if (pendingInlines == null) {
+ pendingInlines = new java.util.ArrayList(5);
+ }
+ pendingInlines.add(new PendingInline(fo, firstWhiteSpaceInSeq));
+ }
+
private class EOLchecker {
private boolean nextIsEOL = false;
private RecursiveCharIterator charIter;
- EOLchecker(RecursiveCharIterator charIter) {
- this.charIter = charIter;
+ EOLchecker(CharIterator charIter) {
+ this.charIter = (RecursiveCharIterator) charIter;
}
boolean beforeLinefeed() {
@@ -241,6 +270,16 @@
void reset() {
nextIsEOL = false;
+ }
+ }
+
+ private class PendingInline {
+ protected FObjMixed fo;
+ protected CharIterator firstTrailingWhiteSpace;
+
+ PendingInline(FObjMixed fo, CharIterator firstTrailingWhiteSpace) {
+ this.fo = fo;
+ this.firstTrailingWhiteSpace = firstTrailingWhiteSpace;
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 02.02.2006 12:37:13 Manuel Mall wrote:
> On Thursday 02 February 2006 16:49, Jeremias Maerki wrote:
> > My gut feeling here is that it is now more correct than before but
> > there are still two trailing spaces in the area tree that I'd
> > expected not to be there. But then, I still haven't done my homework
> > concerning whitespace handling. :-(
>
> Jeremias,
>
> looks good to me - which two spaces are you unsure about?
My fault. I did an "svn up" from the command-line and didn't refresh the
sources in Eclipse. Everything's in order.
Jeremias Maerki
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Manuel Mall <mm...@arcus.com.au>.
On Thursday 02 February 2006 16:49, Jeremias Maerki wrote:
> My gut feeling here is that it is now more correct than before but
> there are still two trailing spaces in the area tree that I'd
> expected not to be there. But then, I still haven't done my homework
> concerning whitespace handling. :-(
Jeremias,
looks good to me - which two spaces are you unsure about?
>
> On 01.02.2006 22:14:11 Andreas L Delmelle wrote:
> > On Feb 1, 2006, at 21:51, Andreas L Delmelle wrote:
> > > ...
> > > Alright! The space has finally disappeared, but...
> > > I still end up with the same failing testcase:
> > > inline_border_padding.xml
> > >
> > > If you look at the sixth block, somehow an ipd of 118.83pt is
> > > expected, but I currently get an ipd of 113.27pt. Can anyone
> > > explain where the expected value comes from? If I look at the
> > > resulting area tree, then I'm under the impression that the
> > > output is A-OK... Am I missing something?
> > > May it be altered to 113.27pt? (= *my* expectation :-))
> > >
> > > Anyway, I'm reluctant to commit until this one final riddle is
> > > solved.
> >
> > On second thought, I'm just going to change the expected value...
> > What's 5.56/72 (+/- 2mm) anyway? Right! Peanuts.
> >
> > If anyone feels differently, they're most welcome to explain why
> > :-)
> >
Andreas,
the test was not written to check whitespace handling. Therefore the ipd
test was based on the incorrect whitespace present before. You now
correctly remove the two trailing spaces (2780mpt * 2) and therefore
the ipd shrinks by 5560mpt. IMO my opinion all is fine and yes you had
to adjust the test.
> > Cheers,
> >
> > Andreas
>
> Jeremias Maerki
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
My gut feeling here is that it is now more correct than before but there
are still two trailing spaces in the area tree that I'd expected not to
be there. But then, I still haven't done my homework concerning
whitespace handling. :-(
On 01.02.2006 22:14:11 Andreas L Delmelle wrote:
> On Feb 1, 2006, at 21:51, Andreas L Delmelle wrote:
>
> > ...
> > Alright! The space has finally disappeared, but...
> > I still end up with the same failing testcase:
> > inline_border_padding.xml
> >
> > If you look at the sixth block, somehow an ipd of 118.83pt is
> > expected, but I currently get an ipd of 113.27pt. Can anyone
> > explain where the expected value comes from? If I look at the
> > resulting area tree, then I'm under the impression that the output
> > is A-OK... Am I missing something?
> > May it be altered to 113.27pt? (= *my* expectation :-))
> >
> > Anyway, I'm reluctant to commit until this one final riddle is solved.
>
> On second thought, I'm just going to change the expected value...
> What's 5.56/72 (+/- 2mm) anyway? Right! Peanuts.
>
> If anyone feels differently, they're most welcome to explain why :-)
>
> Cheers,
>
> Andreas
Jeremias Maerki
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Feb 1, 2006, at 21:51, Andreas L Delmelle wrote:
> ...
> Alright! The space has finally disappeared, but...
> I still end up with the same failing testcase:
> inline_border_padding.xml
>
> If you look at the sixth block, somehow an ipd of 118.83pt is
> expected, but I currently get an ipd of 113.27pt. Can anyone
> explain where the expected value comes from? If I look at the
> resulting area tree, then I'm under the impression that the output
> is A-OK... Am I missing something?
> May it be altered to 113.27pt? (= *my* expectation :-))
>
> Anyway, I'm reluctant to commit until this one final riddle is solved.
On second thought, I'm just going to change the expected value...
What's 5.56/72 (+/- 2mm) anyway? Right! Peanuts.
If anyone feels differently, they're most welcome to explain why :-)
Cheers,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Feb 1, 2006, at 19:50, Andreas L Delmelle wrote:
>> ...
>> I'll see if I get to debugging this further. Stay tuned.
>
> No real debugging done yet. I just commented out the mentioned
> conditional in TextLM, so eventual remaining spaces are always
> mapped to the no-BAP element list. The result is identical, so ...
> back to square one.
Alright! The space has finally disappeared, but...
I still end up with the same failing testcase: inline_border_padding.xml
If you look at the sixth block, somehow an ipd of 118.83pt is
expected, but I currently get an ipd of 113.27pt. Can anyone explain
where the expected value comes from? If I look at the resulting area
tree, then I'm under the impression that the output is A-OK... Am I
missing something?
May it be altered to 113.27pt? (= *my* expectation :-))
Anyway, I'm reluctant to commit until this one final riddle is solved.
Cheers,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Feb 1, 2006, at 18:34, Andreas L Delmelle wrote:
> On Feb 1, 2006, at 02:25, Manuel Mall wrote:
>
>> If BAP != 0 then a Knuth box element is inserted. This acts as a
>> fence for
>> removeTrailingSpaces(), that is removeTrailingSapces() cannot
>> distinguish
>> if the Box represents an actual word in the text or is a border/
>> padding
>> space reservation and stops removal once it sees a Box. So, yes
>> IMO your
>> algorithm does not produce a proper result but removeTrailingSpaces()
>> hides this unless we have border/padding intervening.
>
> Not sure if I got this correctly, but LineLM.removeTrailingSpaces()
> has been completely removed/disabled in my local sandbox, so this
> box doesn't serve any purpose currently (?)
> That's exactly why this is puzzling me... In one case, no borders/
> padding, it does produce a correct result --layout doesn't remove
> trailing spaces anymore, and in the no-BAP case it appears it
> doesn't have to-- while in the other a space remains.
>
> I'll see if I get to debugging this further. Stay tuned.
No real debugging done yet. I just commented out the mentioned
conditional in TextLM, so eventual remaining spaces are always mapped
to the no-BAP element list. The result is identical, so ... back to
square one.
Later,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Feb 1, 2006, at 02:25, Manuel Mall wrote:
>
> If BAP != 0 then a Knuth box element is inserted. This acts as a
> fence for
> removeTrailingSpaces(), that is removeTrailingSapces() cannot
> distinguish
> if the Box represents an actual word in the text or is a border/
> padding
> space reservation and stops removal once it sees a Box. So, yes IMO
> your
> algorithm does not produce a proper result but removeTrailingSpaces()
> hides this unless we have border/padding intervening.
Not sure if I got this correctly, but LineLM.removeTrailingSpaces()
has been completely removed/disabled in my local sandbox, so this box
doesn't serve any purpose currently (?)
That's exactly why this is puzzling me... In one case, no borders/
padding, it does produce a correct result --layout doesn't remove
trailing spaces anymore, and in the no-BAP case it appears it doesn't
have to-- while in the other a space remains.
I'll see if I get to debugging this further. Stay tuned.
Cheers,
Andreas
Re: svn commit: r367395 -
/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler
.java
Posted by Manuel Mall <mm...@arcus.com.au>.
> On Jan 31, 2006, at 06:12, Manuel Mall wrote:
>
>>
>> Just curious if you had a chance to have a look yet.
>
> Errm... A look yes, but I haven't found the time to correct it yet.
> BTW: does anyone know whether the removeTrailingSpaces() method was
> the only part of layout concerned with space removal? The funny thing
> is that if I remove the border and padding props, the remaining space
> disappears. If that is so, then AFAICT, it's possible that my
> algorithm still doesn't produce the proper result, but layout
> corrects this (unless there's border-and-padding involved --which,
> IIC, leads to a different element list being constructed for spaces
> (?) see TextLayoutManager at line 832 "if (lineStartBAP != 0 ||
> lineEndBAP != 0)"; I presume BAP is short for Border And Padding)
>
If BAP != 0 then a Knuth box element is inserted. This acts as a fence for
removeTrailingSpaces(), that is removeTrailingSapces() cannot distinguish
if the Box represents an actual word in the text or is a border/padding
space reservation and stops removal once it sees a Box. So, yes IMO your
algorithm does not produce a proper result but removeTrailingSpaces()
hides this unless we have border/padding intervening.
> Cheers,
>
> Andreas
>
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler .java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 31, 2006, at 06:12, Manuel Mall wrote:
>
> Just curious if you had a chance to have a look yet.
Errm... A look yes, but I haven't found the time to correct it yet.
BTW: does anyone know whether the removeTrailingSpaces() method was
the only part of layout concerned with space removal? The funny thing
is that if I remove the border and padding props, the remaining space
disappears. If that is so, then AFAICT, it's possible that my
algorithm still doesn't produce the proper result, but layout
corrects this (unless there's border-and-padding involved --which,
IIC, leads to a different element list being constructed for spaces
(?) see TextLayoutManager at line 832 "if (lineStartBAP != 0 ||
lineEndBAP != 0)"; I presume BAP is short for Border And Padding)
Cheers,
Andreas
Re: svn commit: r367395 -
/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler
.java
Posted by Manuel Mall <mm...@arcus.com.au>.
> On Jan 13, 2006, at 15:47, Manuel Mall wrote:
>
>>> Applied the diff but it didn't change my observation in the debugger.
>>> The extra space you noticed is coming from the FO. Therefore IMO the
>>> patch is not quite doing what you would like it to do. I haven't
>>> looked at your code yet.
>>>
>> Andreas,
>>
>> The following change in XMLWhiteSpaceHandler
>> && (/*nonWhiteSpacePresent ||*/ nextChild !=
>> null))
>> seems to fix it.
>
> OK. I'll have a closer look at it this weekend....
>
Just curious if you had a chance to have a look yet.
> Cheers,
>
> Andreas
>
Regards
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 13, 2006, at 15:47, Manuel Mall wrote:
>> Applied the diff but it didn't change my observation in the debugger.
>> The extra space you noticed is coming from the FO. Therefore IMO the
>> patch is not quite doing what you would like it to do. I haven't
>> looked at your code yet.
>>
> Andreas,
>
> The following change in XMLWhiteSpaceHandler
> && (/*nonWhiteSpacePresent ||*/ nextChild !=
> null))
> seems to fix it.
OK. I'll have a closer look at it this weekend....
Cheers,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Manuel Mall <mm...@arcus.com.au>.
On Fri, 13 Jan 2006 08:01 am, Manuel Mall wrote:
> On Fri, 13 Jan 2006 12:49 am, Andreas L Delmelle wrote:
> > On Jan 12, 2006, at 01:14, Manuel Mall wrote:
> > > On Thu, 12 Jan 2006 05:34 am, Andreas L Delmelle wrote:
> > > <snip />
> > > without your patch I cannot really replicate this. When I run
> > > through the debugger now I can see the space being given from the
> > > FO to the TextLayoutManager (look at the textarray in the TextLM
> > > constructor) therefore to me its still a refinement issue. But if
> > > your latest modifications fixes that then I would need that patch
> > > to further investigate.
> >
> > Try the diff in attach (modifications to XMLWhiteSpaceHandler and
> > LineLayoutManager)
>
> Applied the diff but it didn't change my observation in the debugger.
> The extra space you noticed is coming from the FO. Therefore IMO the
> patch is not quite doing what you would like it to do. I haven't
> looked at your code yet.
>
Andreas,
The following change in XMLWhiteSpaceHandler
&& (/*nonWhiteSpacePresent ||*/ nextChild != null))
seems to fix it. That is I just commented out the test for
nonWhiteSpacePresent. I do not fully understand the implications of
this change. I'll leave it up to you to further explore.
> Manuel
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Manuel Mall <mm...@arcus.com.au>.
On Fri, 13 Jan 2006 12:49 am, Andreas L Delmelle wrote:
> On Jan 12, 2006, at 01:14, Manuel Mall wrote:
> > On Thu, 12 Jan 2006 05:34 am, Andreas L Delmelle wrote:
> > <snip />
> > without your patch I cannot really replicate this. When I run
> > through the debugger now I can see the space being given from the
> > FO to the TextLayoutManager (look at the textarray in the TextLM
> > constructor) therefore to me its still a refinement issue. But if
> > your latest modifications fixes that then I would need that patch
> > to further investigate.
>
> Try the diff in attach (modifications to XMLWhiteSpaceHandler and
> LineLayoutManager)
>
Applied the diff but it didn't change my observation in the debugger.
The extra space you noticed is coming from the FO. Therefore IMO the
patch is not quite doing what you would like it to do. I haven't looked
at your code yet.
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 12, 2006, at 01:14, Manuel Mall wrote:
> On Thu, 12 Jan 2006 05:34 am, Andreas L Delmelle wrote:
> <snip />
> without your patch I cannot really replicate this. When I run through
> the debugger now I can see the space being given from the FO to the
> TextLayoutManager (look at the textarray in the TextLM constructor)
> therefore to me its still a refinement issue. But if your latest
> modifications fixes that then I would need that patch to further
> investigate.
Try the diff in attach (modifications to XMLWhiteSpaceHandler and
LineLayoutManager)
>
>>
>> Also, I got an error on testcase table_width.xml:
>>
>> [junit] Testcase: table_width.xml
>> (org.apache.fop.layoutengine.LayoutEngineTestSuite$1): Caused an
>> ERROR
>> [junit] java.lang.IndexOutOfBoundsException: Index: 6, Size: 6
>
> Do we know which patch (SVN revision) introduced this issue?
Must have something to do with the revised white-space handling or
the removal of removeElementsForTrailingSpaces(). Before that, I
never received the error. AFAIU, if this error occurs, this would
mean that a given sub-sequence contains no boxes (only glues/
penalties), while the algorithm seems to always expect at least one
box...
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Manuel Mall <mm...@arcus.com.au>.
On Thu, 12 Jan 2006 05:34 am, Andreas L Delmelle wrote:
> On Jan 11, 2006, at 17:56, Andreas L Delmelle wrote:
> > On Jan 10, 2006, at 14:02, Manuel Mall wrote:
> >> I got the lastest svn version of fop and then commented out the
> >> removeElementsForTrailingSpaces() method in LineLayoutManager as
> >> theoretically your patch should make this unnecessary. However, we
> >> get
> >> a erroneous trailing space in the block_white-space-collapse_1.xml
> >> test
> >> case.
> >
> > Just a quick FYI: I think I've found the problem, and almost solved
> > it... Almost, since now the mentioned testcase works fine, but in
> > inline_border_padding.xml, I now get an error, and the reason for
> > it completely eludes me. On the one hand, the error indicates an
> > inlineParent area with an IPD that is *less* than the expected
> > value. On the other hand, that same inlineParent apparently has an
> > offending trailing space area --while the conditions in the FOTree
> > are identical AFAICT (it's the last inline in the last block).
> >
> > Once I succeed in tracking this one down, I'll commit again.
>
> OK. So apparently, this has nothing to do with refinement
> white-space- handling, IIC.
>
> I tried commenting out removeElementsForTrailingSpaces() and ran a FO
> containing the following two blocks:
>
> <fo:block background-color="silver" margin="1pt 0pt 1pt 0pt">
> <fo:inline background-color="orange">
> inline level
> <fo:inline background-color="red">
> nested inline level
> </fo:inline>
> </fo:inline>
> </fo:block>
> <fo:block background-color="silver" margin="3pt 0pt 3pt 0pt">
> Demonstrates nested
> <fo:inline background-color="yellow" border="solid 2pt red"
> padding-start="2pt" padding-end="2pt" >inlines
> <fo:inline background-color="orange" border="solid 1pt green"
> padding-start="2pt" padding-end="2pt" >finishing together
> </fo:inline>
> </fo:inline>
> </fo:block>
>
> and for the two inner inlines, I get the following area tree
> fragments:
>
> <inlineparent ...>
> <text ...>
> <space offset="0"> </space>
> <word offset="0">nested</word>
> <space offset="0"> </space>
> <word offset="0">inline</word>
> <space offset="0"> </space>
> <word offset="0">level</word>
> </text>
> </inlineparent>
>
> <inlineparent ...>
> <text ...>
> <word offset="0">finishing</word>
> <space offset="0"> </space>
> <word offset="0">together</word>
> <space offset="0"> </space>
> </text>
> </inlineparent>
>
> If I remove the border-* and padding-* properties, the trailing space
> area for the latter disappears...?
>
> Of course, the expected result could simply be modified, but it would
> be much better if this were fixed in the related code. No idea where
> precisely.
>
> => Q: Disable it FTM, or alter the expectation to make it pass?
Andreas,
without your patch I cannot really replicate this. When I run through
the debugger now I can see the space being given from the FO to the
TextLayoutManager (look at the textarray in the TextLM constructor)
therefore to me its still a refinement issue. But if your latest
modifications fixes that then I would need that patch to further
investigate.
>
> Also, I got an error on testcase table_width.xml:
>
> [junit] Testcase: table_width.xml
> (org.apache.fop.layoutengine.LayoutEngineTestSuite$1): Caused an
> ERROR
> [junit] java.lang.IndexOutOfBoundsException: Index: 6, Size: 6
Do we know which patch (SVN revision) introduced this issue?
<snip />
> Cheers,
>
> Andreas
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Chris Bowditch <bo...@hotmail.com>.
Andreas L Delmelle wrote:
> On Jan 11, 2006, at 17:56, Andreas L Delmelle wrote:
<snip/>
>
> OK. So apparently, this has nothing to do with refinement white-space-
> handling, IIC.
<snip/>
>
> If I remove the border-* and padding-* properties, the trailing space
> area for the latter disappears...?
>
> Of course, the expected result could simply be modified, but it would
> be much better if this were fixed in the related code. No idea where
> precisely.
>
> => Q: Disable it FTM, or alter the expectation to make it pass?
I don't like the sound of this at all. May I suggest an alternative
approach to finding the cause of the problem? Could you create a diff of
all changed files and post it up here, so others may aid in the
investigation?
<snip/>
> I changed the related code in BreakingAlgorithm from:
>
> while(alignment != EN_CENTER
> && ! ((KnuthElement) par.get(firstBoxIndex)).isBox()) {
> firstBoxIndex++;
> ...
>
> to:
>
> if(alignment != EN_CENTER
> while(par.size() > firstBoxIndex
> && ! ((KnuthElement) par.get(firstBoxIndex)).isBox()) {
> firstBoxIndex++;
> ...
>
> That solved this particular little problem, but it is more of a quick
> fix, I guess.
I think we need to understand the root cause of this before making this
change. Unfortunately Jeremias isn't around for the next 2/3 days, so
may I suggest holding off making this change until he returns. He has a
very good understand of this bit of code ;)
Chris
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 11, 2006, at 17:56, Andreas L Delmelle wrote:
> On Jan 10, 2006, at 14:02, Manuel Mall wrote:
>
>> I got the lastest svn version of fop and then commented out the
>> removeElementsForTrailingSpaces() method in LineLayoutManager as
>> theoretically your patch should make this unnecessary. However, we
>> get
>> a erroneous trailing space in the block_white-space-collapse_1.xml
>> test
>> case.
>
> Just a quick FYI: I think I've found the problem, and almost solved
> it... Almost, since now the mentioned testcase works fine, but in
> inline_border_padding.xml, I now get an error, and the reason for
> it completely eludes me. On the one hand, the error indicates an
> inlineParent area with an IPD that is *less* than the expected
> value. On the other hand, that same inlineParent apparently has an
> offending trailing space area --while the conditions in the FOTree
> are identical AFAICT (it's the last inline in the last block).
>
> Once I succeed in tracking this one down, I'll commit again.
OK. So apparently, this has nothing to do with refinement white-space-
handling, IIC.
I tried commenting out removeElementsForTrailingSpaces() and ran a FO
containing the following two blocks:
<fo:block background-color="silver" margin="1pt 0pt 1pt 0pt">
<fo:inline background-color="orange">
inline level
<fo:inline background-color="red">
nested inline level
</fo:inline>
</fo:inline>
</fo:block>
<fo:block background-color="silver" margin="3pt 0pt 3pt 0pt">
Demonstrates nested
<fo:inline background-color="yellow" border="solid 2pt red"
padding-start="2pt" padding-end="2pt" >inlines
<fo:inline background-color="orange" border="solid 1pt green"
padding-start="2pt" padding-end="2pt" >finishing together
</fo:inline>
</fo:inline>
</fo:block>
and for the two inner inlines, I get the following area tree fragments:
<inlineparent ...>
<text ...>
<space offset="0"> </space>
<word offset="0">nested</word>
<space offset="0"> </space>
<word offset="0">inline</word>
<space offset="0"> </space>
<word offset="0">level</word>
</text>
</inlineparent>
<inlineparent ...>
<text ...>
<word offset="0">finishing</word>
<space offset="0"> </space>
<word offset="0">together</word>
<space offset="0"> </space>
</text>
</inlineparent>
If I remove the border-* and padding-* properties, the trailing space
area for the latter disappears...?
Of course, the expected result could simply be modified, but it would
be much better if this were fixed in the related code. No idea where
precisely.
=> Q: Disable it FTM, or alter the expectation to make it pass?
Also, I got an error on testcase table_width.xml:
[junit] Testcase: table_width.xml
(org.apache.fop.layoutengine.LayoutEngineTestSuite$1): Caused an
ERROR
[junit] java.lang.IndexOutOfBoundsException: Index: 6, Size: 6
[junit] ; SystemID: file:///Developer/javatools/xml-fop/test/
layoutengine/testcase2fo.xsl; Line#: 34; Column#: 60
[junit] javax.xml.transform.TransformerException:
java.lang.IndexOutOfBoundsException: Index: 6, Size: 6
...
[junit] Caused by: java.lang.IndexOutOfBoundsException: Index: 6,
Size: 6
[junit] at java.util.ArrayList.RangeCheck(ArrayList.java:507)
[junit] at java.util.ArrayList.get(ArrayList.java:324)
[junit] at
org.apache.fop.layoutmgr.BreakingAlgorithm.findBreakingPoints
(BreakingAlgorithm.java:367)
[junit] at
org.apache.fop.layoutmgr.BreakingAlgorithm.findBreakingPoints
(BreakingAlgorithm.java:339)
[junit] at org.apache.fop.layoutmgr.inline.LineLayoutManager
$LineBreakingAlgorithm.findBreakingPoints
(LineLayoutManager.java:537)
[junit] at org.apache.fop.layoutmgr.inline.LineLayoutManager
.findOptimalBreakingPoints(LineLayoutManager.java:1000)
I changed the related code in BreakingAlgorithm from:
while(alignment != EN_CENTER
&& ! ((KnuthElement) par.get(firstBoxIndex)).isBox()) {
firstBoxIndex++;
...
to:
if(alignment != EN_CENTER
while(par.size() > firstBoxIndex
&& ! ((KnuthElement) par.get(firstBoxIndex)).isBox()) {
firstBoxIndex++;
...
That solved this particular little problem, but it is more of a quick
fix, I guess.
All other tests pass, so if my above tiny question is answered, and
no-one objects to the described change in BreakingAlgorithm.java,
I'll do the commit.
Cheers,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 10, 2006, at 14:02, Manuel Mall wrote:
> I got the lastest svn version of fop and then commented out the
> removeElementsForTrailingSpaces() method in LineLayoutManager as
> theoretically your patch should make this unnecessary. However, we get
> a erroneous trailing space in the block_white-space-collapse_1.xml
> test
> case.
Just a quick FYI: I think I've found the problem, and almost solved
it... Almost, since now the mentioned testcase works fine, but in
inline_border_padding.xml, I now get an error, and the reason for it
completely eludes me. On the one hand, the error indicates an
inlineParent area with an IPD that is *less* than the expected value.
On the other hand, that same inlineParent apparently has an offending
trailing space area --while the conditions in the FOTree are
identical AFAICT (it's the last inline in the last block).
Once I succeed in tracking this one down, I'll commit again.
Cheers,
Andreas
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Manuel Mall <mm...@arcus.com.au>.
On Tue, 10 Jan 2006 05:24 am, Andreas L Delmelle wrote:
> On Jan 9, 2006, at 22:20, adelmelle@apache.org wrote:
> > Author: adelmelle
> > Date: Mon Jan 9 13:20:07 2006
> > New Revision: 367395
> >
> > URL: http://svn.apache.org/viewcvs?rev=367395&view=rev
>
> OK folks,
>
> This should solve the 'trailing white-space for trailing (nested)
> inlines' issue recently discussed ad nauseam here :-)
>
I got the lastest svn version of fop and then commented out the
removeElementsForTrailingSpaces() method in LineLayoutManager as
theoretically your patch should make this unnecessary. However, we get
a erroneous trailing space in the block_white-space-collapse_1.xml test
case.
It seems the nested inline trailing space removal in the case of
<fo:block background-color="silver" margin="1pt 0pt 1pt 0pt">
<fo:inline background-color="orange">
inline level
<fo:inline background-color="red">
nested inline level
</fo:inline>
</fo:inline>
</fo:block>
is not yet quite right unless I misinterpret the results.
<snip/>
>
> Cheers,
>
> Andreas
Regards
Manuel
Re: svn commit: r367395 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/XMLWhiteSpaceHandler.java
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jan 9, 2006, at 22:20, adelmelle@apache.org wrote:
> Author: adelmelle
> Date: Mon Jan 9 13:20:07 2006
> New Revision: 367395
>
> URL: http://svn.apache.org/viewcvs?rev=367395&view=rev
OK folks,
This should solve the 'trailing white-space for trailing (nested)
inlines' issue recently discussed ad nauseam here :-)
I was a bit doubtful at first about using mark() for this, since I
received a couple of concurrent modification errors. Those turned out
to be caused by blocks being added to the pendingInlines list, so
concurrent modification of the underlying list of white-space was to
be expected. Once this was fixed, all regression tests passed w/o
complications. It should work since *iff* there is remaining white-
space (inWhiteSpace==true), then the element at which the iterator
was marked, is guaranteed to be present (and the rest of the list
unchanged if white-space-collapse="false").
Again, no noticeable change in the iterators yet. Strictly speaking,
at least the InlineCharIterator could already be removed, I think
(not used anymore), but I consider the iterators a task in itself, so
will take care of it then and there.
Markers, as Manuel suggested, are currently still white-space-treated
according to their possibly erroneous property values, since that
would still cover most of the use-cases. Obvious work-around is to
use a block simply as a container for the related properties to be
inherited by the marker-content.
Cheers,
Andreas