Bug 38087 - [patch] force-page-count property implementation
Summary: [patch] force-page-count property implementation
Status: CLOSED FIXED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: page-master/layout (show other bugs)
Version: trunk
Hardware: Other other
: P2 enhancement
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-01 13:42 UTC by gerhard oettl
Modified: 2012-04-01 06:39 UTC (History)
0 users



Attachments
the patch itself (6.81 KB, text/plain)
2006-01-01 13:44 UTC, gerhard oettl
Details
this fo i used for testing (no automated test case) (9.91 KB, text/plain)
2006-01-01 13:45 UTC, gerhard oettl
Details
patch 2 (using area tree) (8.26 KB, patch)
2006-01-04 23:59 UTC, gerhard oettl
Details | Diff
a better fo example file (10.76 KB, text/plain)
2006-01-05 12:14 UTC, gerhard oettl
Details
testcase for force-page-count="auto" (4.87 KB, text/plain)
2006-01-07 12:32 UTC, gerhard oettl
Details
testcase for all but auto (5.79 KB, text/plain)
2006-01-07 12:34 UTC, gerhard oettl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description gerhard oettl 2006-01-01 13:42:48 UTC
This patch (re)implements the force-page-count property.

I tried to implement it with as little changes to the
code as possible. Therefore some design issues and questions
remain - see later.

Description:
The force-page-count property is handled by adding a method
to the PageSequenceLayoutManger, because there are is a
method that has all informations (static content, pagenumber,
...) about creating a new page (makeNewPage).
The main problem was that the force-page-count property needs
the initial-page-number of the next page-sequence which does
not exist at this time.
I solved this by preserving the current PageSequenceLayoutManger
after the end of the page-sequence. I used the Root object
for this because the last pagenumber of the sequence and other
informations are also handed over this object from one
page-sequence to the other. At the beginning of the following
page-sequence the (stored) previous PageSequenceLayoutManger
is called with the now current PageSequence object to finish
this work.

Changes to Root.java:
- insert the variable previousPageSeqLM to store a
  PageSequenceLayoutManger for later use
  (and import the necessary class definition).

Changes to PageSequenceLayoutManger.java:
- split off the last part of activateLayout into a separate
  method (finishLeayout) because i need the same functionality
  after the force-page-count handling.
- preserve the current PageSequenceLayoutManger object to the
  Root object at the end of the finishLayout method.
- implement the force-page-count propterty in the
  checkForcePageCount method. This method needs the next
  PageSequence object as parameter. To get the initial-page-number
  property of this next page-sequence i copied the code from
  PageSequence.initPageNumber.

Changes to PageSequence.java:
- call the checkForcePageCount method of the preserved previous
  PageSequenceLayoutManger inside the startOfNode method
  immediately before initPageNumber so that this method is
  waware of possibly inserted pages of the previous page-sequence.
- insert a getFocePageCount method to get the private variable.
- insert a getInitialPageNumber method to get the private variable.


Open issues / questions:
- i think it very likely that the previousPageSeqLM variable of
  the Root class should be privat and there should be a
  setPreviousPageSeqLM and a getPreviousPageSeqLM method to
  hand the variable into and out of the object.
- there was a second possiblity to implement this all
  by using the AreaTreeHandler:
  * instead of saving the PageSequenceLayoutManger object
    at the end of the finishLayout method of the
    PageSequenceLayoutManger it could be preserved at the
    end of the endPageSequence method of the AreaTreeHandler.
  * instead of calling the checkForcePageCount from the
    startOfNode method of PageSequence it could be called
    from the startPageSequence method of the AreaTreeHandler
    after receiving the rootFObj.
  * all other could be left nearly as with this patch with
    the exception that the startingPageNumber of the PageSequence
    has to be recalced after calling the checkForcePageCount
    because it is already calced at this point and does not
    know about possibly inserted pages from the
    checkForcePageCount method before.
  someone who has a more overall sight of the code has to
  decide where it fits best.




I know the force-page-count property is of low priority, but
it would be nice to get feedback, if someone can spare time
to have a look at this patch.


I hope i was not too verbose ;-) and that my patch is more
usefull than the review is time-consuming and that i dont
violate too much design guidelines.


happy new year
gerhard
Comment 1 gerhard oettl 2006-01-01 13:44:39 UTC
Created attachment 17303 [details]
the patch itself
Comment 2 gerhard oettl 2006-01-01 13:45:33 UTC
Created attachment 17304 [details]
this fo i used for testing (no automated test case)
Comment 3 Simon Pepping 2006-01-03 22:23:49 UTC
Gerhard,

Thanks for the patch. It looks fine.

I am not quite happy with the fact that checkForcePageCount is called
by the next page sequence. This is an interaction between page
sequences, and it is better dealt with by the controlling structure,
in this case AreaTreeHandler. In other words, I go with your
suggestion in the second *.

I also note that the object that is passed to checkForcePageCount can
be narrowed down to Numeric nextPageSeq.getInitialPageNumber().

There is a little error in the last page sequence but one in your demo
file. It says that next is auto-even, which is not true.

If you make the change to the patch, I will be happy to commit it.

Simon
Comment 4 gerhard oettl 2006-01-04 23:59:27 UTC
Created attachment 17329 [details]
patch 2 (using area tree)

ad the now 2c)
At the time the checkForcePageCount is called from the
AreaTreeHandler the initialPageNumber is already calced by the
PageSequence without knowing about eventueally later added pages so 
it must be recalced. I did this by making the initPageNumber method
public. If this should stay private i have to add a public
recalcInitPageNumber method that does nothing than call the
private initPageNumber method - at your decision.
Comment 5 gerhard oettl 2006-01-05 12:14:49 UTC
Created attachment 17332 [details]
a better fo example file
Comment 6 Simon Pepping 2006-01-06 22:33:16 UTC
Applied. I rearranged the calls somewhat. This is the resulting call hierarchy:

PageSequence.startOfNode
 AreaTreeHandler.startPageSequence
  PSLM.doForcePageCount (previous PS if not null)
   PSLM.finishPage
  PSLM.finishPageSequence (previous PS if not null)
   Root.notifyPageSequenceFinished
   AreaTreeHandler.notifyPageSequenceFinished
  PageSequence.initPageNumber

PageSequence.endOfNode
 AreaTreeHandler.endPageSequence
  PSLM.activateLayout
   PSLM.finishPage

Thanks for the patch.

Simon
Comment 7 gerhard oettl 2006-01-07 12:32:05 UTC
Created attachment 17356 [details]
testcase for force-page-count="auto"
Comment 8 gerhard oettl 2006-01-07 12:34:35 UTC
Created attachment 17357 [details]
testcase for all but auto

I started with pagenumber 201 to allow to concatenate to the testcase for
force-page-count="auto"
Comment 9 Simon Pepping 2006-01-09 22:20:26 UTC
Thanks for the fine test cases. Re noauto: Do you really expect 18 pages? I get
17, and I think that is correct.
Comment 10 gerhard oettl 2006-01-10 00:38:39 UTC
Yes i expect 17. I want to show the missing pageno 217 - a typical case of
self-delusion. 
Comment 11 Simon Pepping 2006-01-10 21:21:47 UTC
Committed the test cases. Thanks. Simon
Comment 12 Glenn Adams 2012-04-01 06:39:38 UTC
batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed