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 Jan Tosovsky <j....@email.cz> on 2015/07/02 00:42:04 UTC

RE: Change bars status

I've attached the updated patch recently to the related ticket:
https://issues.apache.org/jira/browse/FOP-1760

>From that time I locally fixed two issues (2 - support for overlapping bars)
and (4 - wrapping table cells). The rest requires the help of more
experienced devs.

Btw, how about creating a dedicated branch and tweaking it separately?

Thanks, Jan

Re: Change bars status

Posted by Clay Leeds <th...@gmail.com>.
> No real preference here. Your suggestion is OK. 
> In general, in typesetting, a "lorem ipsum" generator is frequently used to generate dummy text (for 'simple' Latin scripts, that is). See: http://www.lipsum.com
> 
> KR
> 
> Andreas

Heh...

You can get a similar output using the 'Classic Ipsum' setting on this site:

http://slipsum.com

...but that's not nearly as much fun...

Then there's always:

http://buseyipsum.com

Web Maestro Clay



Re: Change bars status

Posted by Andreas Delmelle <an...@telenet.be>.
Hi Jan

Thanks for your efforts so far in getting this moving forward!

As to your question:

> <snip />
> Btw, what is preferred dummy text? Currently I use The Universal Declaration
> of Human Rights (Article 1). Is it Ok?

No real preference here. Your suggestion is OK. 
In general, in typesetting, a "lorem ipsum" generator is frequently used to generate dummy text (for 'simple' Latin scripts, that is). See: http://www.lipsum.com

KR

Andreas

RE: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
On 2015-07-02 Jan Tosovsky wrote:
> I've attached the updated patch recently to the related ticket:
> https://issues.apache.org/jira/browse/FOP-1760
> 
> From that time I locally fixed two issues (2 - support for overlapping
> bars) and (4 - wrapping table cells). The rest requires the help of
> more experienced devs.

I've moved forward the rest of issues. Currently I am considering what
should be covered by layout tests.

My proposal:
change-bar-on-inline (solid bar of same thickness in various places of the
content)
change-bar-on-block (same for block element)
change-bar-on-list (same for list)
change-bar-on-table (same for various table fragments - cell, rows, headers,
tables)
change-bar-on-wrapper (same for wrapper element)
change-bar-on-overlapped (testing overlapped bars)
change-bar-on-mixed

change-bar-style (block elements with various bar styles
/solid/dashed/dotted/..., colors and widths)
change-bar-position (block elements with start/left/end/inset/... position)
change-bar-direction (testing position in LR/RL direction)

Btw, what is preferred dummy text? Currently I use The Universal Declaration
of Human Rights (Article 1). Is it Ok?

Thanks, Jan


Re: Change bars status

Posted by "Andreas L. Delmelle" <an...@telenet.be>.
Hi Stephan

Thanks for chiming in!

> On 18 Jul 2015, at 11:58, Stephan Thesing <th...@gmx.de> wrote:
> 
> just a few remarks for the changebars:
> - the patch was done when bidi support was starting.  Actually, the first patch contained already provisions for the writing direction to place
>     the bars at the correct edges.   That support was later removed, as bidi support was not working fully working.
> - I have a local patch without respecting writing direction that also produces correct results for the example fo sent earlier in this thread
> - I can provide the patch later this day if there is interest

Sure, if you can attach it in JIRA or make it otherwise available, that would be much appreciated.

Thanks!

KR

Andreas

RE: Re: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
On 2015-07-18 Stephan Thesing wrote:
> I have a local patch without respecting writing direction that also
> produces correct results

wow!

> I can provide the patch later this day if there is interest

Uff :-) I've finalized layout tests and was about submitting a new patch :-)

My current version handles placement depending on writing direction except
inline elements in RL mode. Any improvement in this area is more than
welcome.

I'll submit my changes anyway, it fixes other corner cases. 

You can then grab it, review and finalize it for merging :-)

Thanks, Jan

Re: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
Dear All,

On 2015-07-19 Jan Tosovsky wrote:
> On 2015-07-18 Stephan Thesing wrote:
> > I have a local patch without respecting writing direction that also
> > produces correct results for the example fo sent earlier in this
> > thread
> 
> I like your update as it doesn't touch FOUserAgent (change bars are
> stacked in PageSequence) and it doesn't pollute various area classes
> with that BiDi handling.
> 
> I've incorporated these changes into my clone. For LR writing direction
> it still passes all my tests.
> 
> I'll submit it together with further details to the original JIRA
> issue.

I commited these changes in July: 

https://issues.apache.org/jira/browse/FOP-1760

(together with layout tests and comments)

Would anybody mind reviewing it?

Thanks, Jan



RE: Re: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
Hi Stephan,

On 2015-07-18 Stephan Thesing wrote:
> I have a local patch without respecting writing direction that also
> produces correct results for the example fo sent earlier in this thread

I like your update as it doesn't touch FOUserAgent (change bars are stacked in PageSequence) and it doesn't pollute various area classes with that BiDi handling.

I've incorporated these changes into my clone. For LR writing direction it still passes all my tests.

I'll submit it together with further details to the original JIRA issue.

Thanks a lot for moving this further!

Jan



Aw: Re: Change bars status

Posted by Stephan Thesing <th...@gmx.de>.
Hello,

just a few remarks for the changebars:
 - the patch was done when bidi support was starting.  Actually, the first patch contained already provisions for the writing direction to place
     the bars at the correct edges.   That support was later removed, as bidi support was not working fully working.
 - I have a local patch without respecting writing direction that also produces correct results for the example fo sent earlier in this thread
 - I can provide the patch later this day if there is interest

Best regards
   Stephan
 
 
 

Gesendet: Samstag, 18. Juli 2015 um 11:38 Uhr
Von: "Andreas L. Delmelle" <an...@telenet.be>
An: fop-dev@xmlgraphics.apache.org
Betreff: Re: Change bars status
Hi Jan

> On 16 Jul 2015, at 00:23, Jan Tosovsky <j....@email.cz> wrote:

> It is actually BidiResolver.reorder() method which returns unexpected order.
> It also explains why a period is placed at the beginning of the second line
> even in non-change bars source.

Aha! IIC, the patch predates the implementation of Unicode Bidi resolution, so that may explain a few things.

>
> <snip />
> Is it a known issue or should I file a bug?

Not entirely sure. Seems like it may be related to FOP-2290[*], although that one explicitly mentions empty blocks.
If you are convinced it is a totally different one, feel free to log a new issue.


Thanks!

[*] https://issues.apache.org/jira/browse/FOP-2290


KR

Andreas

Re: Change bars status

Posted by "Andreas L. Delmelle" <an...@telenet.be>.
Hi Jan

> On 16 Jul 2015, at 00:23, Jan Tosovsky <j....@email.cz> wrote:

> It is actually BidiResolver.reorder() method which returns unexpected order.
> It also explains why a period is placed at the beginning of the second line
> even in non-change bars source.

Aha! IIC, the patch predates the implementation of Unicode Bidi resolution, so that may explain a few things.

> 
> <snip />
> Is it a known issue or should I file a bug?

Not entirely sure. Seems like it may be related to FOP-2290[*], although that one explicitly mentions empty blocks. 
If you are convinced it is a totally different one, feel free to log a new issue.


Thanks!

[*] https://issues.apache.org/jira/browse/FOP-2290


KR

Andreas

RE: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
On 2015-07-15 Jan Tosovsky wrote:
> On 2015-07-14 Andreas Delmelle wrote:
> > > On 2015-07-14 Jan Tosovsky wrote:
> > >
> > > Where exactly inline blocks are appended to the list and passed to
> > > the flow?
> >
> > That would be the LayoutManager.addAreas() implementations.
> >
> 
> Thanks for the hint, yes, it comes from
> TextLayoutManager.addAreas()/addMappingAreas();
> 

It is actually BidiResolver.reorder() method which returns unexpected order.
It also explains why a period is placed at the beginning of the second line
even in non-change bars source.

All human beings should act towards one
.another in a spirit of brotherhood

So this order issue seems to be unrelated to the change bars code. My cases
just revealed some uncovered corner cases in the step (4) of the mentioned
method:

// 4. reorder from maximum level to minimum odd level

>From this step on the order is incorrect.

So I tend to give up fine tuning of change bars for RL writing directions
until the above issue is resolved.

Is it a known issue or should I file a bug?

Thanks, Jan


RE: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
On 2015-07-14 Andreas Delmelle wrote:
> > On 2015-07-14 Jan Tosovsky wrote:
> >
> > Where exactly inline blocks are appended to the list and passed to
> > the flow?
> 
> That would be the LayoutManager.addAreas() implementations.
>

Thanks for the hint, yes, it comes from
TextLayoutManager.addAreas()/addMappingAreas();

I didn't touch the original code here, but most likely some side-effect.
I'll investigate later this week.

Jan





Re: Change bars status

Posted by Andreas Delmelle <an...@telenet.be>.
Hi Jan

> On 14 Jul 2015, at 22:40, Jan Tosovsky <j....@email.cz> wrote:
> 
> 
> Where exactly inline blocks are appended to the list and passed to the flow?

That would be the LayoutManager.addAreas() implementations.

I am still not familiar enough with the details of the patch, but if I had to guess, it seems like it could be an issue with the Positions generated for/by the LM, such that the PositionIterator is thrown off when adding the areas for the parent block (?)


KR

Andreas

RE: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
> https://issues.apache.org/jira/browse/FOP-1760

I am testing the code for RL writing mode I am getting unexpected word order
when words or inline elements are wrapped by change bars ('towards' in
example below).

<fo:block>All human beings should act <fo:inline
color="red">towards</fo:inline> one another in a spirit of
brotherhood.</fo:block>

All human beings should act towards one  
    .another in a spirit of brotherhood

<fo:block>All human beings should act <fo:change-bar-begin
change-bar-class="c01" change-bar-style="solid" change-bar-color="red"
change-bar-offset="2mm" change-bar-placement="left"/><fo:inline
color="red">towards</fo:inline><fo:change-bar-end change-bar-class="c01"/>
one another in a spirit of brotherhood.</fo:block>

one towards All human beings should act
    .another in a spirit of brotherhood

Blocks/childAreas are ordered incorrectly already here (AbstractRenderer):
    protected void renderFlow(NormalFlow flow) {
        // the normal flow reference area contains stacked blocks
        List blocks = flow.getChildAreas();
        if (blocks != null) {
            renderBlocks(null, blocks);
        }
    }


Where exactly inline blocks are appended to the list and passed to the flow?


Thanks, Jan

Re: Change bars status

Posted by Andreas Delmelle <an...@telenet.be>.
Hi Jan

> On 10 Jul 2015, at 23:05, Jan Tosovsky <j....@email.cz> wrote:
> 
>> https://issues.apache.org/jira/browse/FOP-1760
> 
> In the original patch there is used Vector for storing ChangeBars
> collection. 
> 
> http://stackoverflow.com/questions/1386275/why-is-java-vector-class-consider
> ed-obsolete-or-deprecated
> 
> I'd suggest replacing it with LinkedList (the order of entries should be
> kept to handle nesting properly). 

Sure, sounds good.

> Or is there any reason for using Vector?

The only "reason" I can think of is that the creator of the patch was used to using that instead of ArrayList or LinkedList.



KR

Andreas

RE: Change bars status

Posted by Jan Tosovsky <j....@email.cz>.
> https://issues.apache.org/jira/browse/FOP-1760

In the original patch there is used Vector for storing ChangeBars
collection. 

http://stackoverflow.com/questions/1386275/why-is-java-vector-class-consider
ed-obsolete-or-deprecated

I'd suggest replacing it with LinkedList (the order of entries should be
kept to handle nesting properly). 

Or is there any reason for using Vector?

Thanks, Jan