[Okular-devel] Review Request: Improve facing pages (center first page) mode when document contains only 1 or 2 pages

[Stephen Anthony]

> On Dec. 15, 2011, 11:40 p.m., Albert Astals Cid wrote:
> > ui/pageview.cpp, line 3735
> > <http://git.reviewboard.kde.org/r/103424/diff/1/?file=43513#file43513line3735>
> >
> >     This looks like an unnecessary change for the bugfix right? Please revert if so

OK, this isn't relevant.


> On Dec. 15, 2011, 11:40 p.m., Albert Astals Cid wrote:
> > ui/pageview.cpp, line 3648
> > <http://git.reviewboard.kde.org/r/103424/diff/1/?file=43513#file43513line3648>
> >
> >     I kind of prefer the centerFirstPage and facingPages to be const still, can you put the pageCount<3 comparison in their assignment lines? Or do you think it impairs readability?

I tried to stick with const, since I also prefer it.  I can probably have const again, but it will require testing the same conditions in multiple places (not just pageCount, but the viewmode as well.  I figured it would be faster to do the tests only once.  I think it might also make the logic harder to follow.


- Stephen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103424/#review8988
-----------------------------------------------------------


On Dec. 15, 2011, 10:45 p.m., Stephen Anthony wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103424/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2011, 10:45 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> This patch improves 'facing pages (center first page)' mode in the case where a document consists of only 1 or 2 pages.  Currently, this mode always assumes two columns of output (which makes sense for 'facing pages').  However, in the case of 1 or 2 page documents, the pages are sized according to two columns, when only one column will ever be used (because of the number of pages present), resulting in pages that don't fill the page in 'Fit to Page' view mode.
> 
> Basically, this patch overrides facing pages in such a case and uses one virtual column, allowing the page to take all available space.
> 
> 
> Diffs
> -----
> 
>   ui/pageview.cpp 78a007a 
> 
> Diff: http://git.reviewboard.kde.org/r/103424/diff/diff
> 
> 
> Testing
> -------
> 
> Testing was done on 1, 2, 3, many page documents in all modes and views.  This patch addresses the specific case where 'facing pages (center first page)' is enabled AND the pagecount is 2 or less.
> 
> 
> Thanks,
> 
> Stephen Anthony
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20111215/7e488575/attachment-0001.html>