[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: dumb stacking page breaker
From: |
Joe Neeman |
Subject: |
Re: dumb stacking page breaker |
Date: |
Fri, 13 Jul 2007 11:45:33 +1000 |
User-agent: |
KMail/1.9.5 |
On Thursday 12 July 2007 07:07, Nicolas Sceaux wrote:
> Joe,
>
> Here is a patch where a stacking-page-breaking function is
> implemented. May you review it, and tell me if there is something wrong
> or some bad coding, before I go on with documentation and regression
> tests? The first tests I have made so far are satisfying. (Also
> suggestions for a better name are welcome:)
The basic idea behind the inheritance in the page-breakers is that the
Page_breaking class looks after the systems and handles
compression/decompression, demerits, etc. The basic operation of a child
class is:
Choose a starting and ending breakpoint and a number of systems. Call
Page_breaking::set_current_breakpoints (or set_to_ideal_line_configuration)
and the Page_breaking class will set some state. In general, there will be
more than one line configuration that fits the constraints, so the child
class can run a bunch of querys on the various configurations (eg. finding
the minimum number of pages, spacing the systems in various ways).
The child class only specifies the index of the configuration and the
Page_breaking class does the rest, including caching things for the case when
the child class runs several different querys on the same configuration
(except that the caching is broken, see below).
To fit with this, I'd suggest that you move
Stacking_page_breaking::compute_page_breaks to something like
Page_breaking::pack_systems_on_least_pages and change its interface to match
the other system-spacing stuff (ie. take a configuration_index instead of the
lines directly and return Spacing_result instead of just the systems per
page). Then you can return the uncompressed/compressed line stuff to private
within Page_breaking.
The looping logic in compute_page_breaks seems to me more convoluted than that
in Page_breaking::min_page_count. As far as I can tell, though, they do
exactly the same thing. Could you change the looping logic to match
min_page_count? Also note that your last "for" loop is unnecessary. As per
the comment in min_page_count, you can do
if (things don't fit on the last page)
{
systems_per_page.back ()--;
systems_per_page.push_back (1);
}
To do the "things don't fit on the last page," it might be worthwhile to add a
Page_spacing::resize_page(Real new_height) method. Then we could reuse it in
Page_breaking::min_page_count.
+ /* what is the purpose of cached_configuration_index_? it is never set,
+ save in clear_line_details_cache -- ns */
Good point, it should be set in the following lines after the comment:
if (cached_configuration_index_ != configuration_index)
{
cached_configuration_index_ = configuration_index;
...
Joe
- dumb stacking page breaker, Nicolas Sceaux, 2007/07/10
- Re: dumb stacking page breaker, Trevor Bača, 2007/07/10
- Re: dumb stacking page breaker, Joe Neeman, 2007/07/10
- Re: dumb stacking page breaker, Nicolas Sceaux, 2007/07/11
- Re: dumb stacking page breaker,
Joe Neeman <=
- Re: dumb stacking page breaker, Nicolas Sceaux, 2007/07/13
- Re: dumb stacking page breaker, Nicolas Sceaux, 2007/07/14
- Re: dumb stacking page breaker, Joe Neeman, 2007/07/14
- Re: dumb stacking page breaker, Nicolas Sceaux, 2007/07/15
- Re: dumb stacking page breaker, Joe Neeman, 2007/07/15