[Okular-devel] Review Request: Introduce currentPageChanged callbacks to DocumentObserver interface

[Tobias Koenig]

> On Aug. 21, 2012, 7:23 p.m., Albert Astals Cid wrote:
> > core/document.cpp, line 2814
> > <http://git.reviewboard.kde.org/r/106058/diff/2/?file=79672#file79672line2814>
> >
> >     I'm wondering if we could save the m_currentPage variable and just use the oldViewport variable as used in the if just above this function.
> >     
> >     It's not that i care about the size of an int, but if we have less "duplicate" information the better, otherwise it's easy for things to get out of sync.

We can't use oldViewport, because of the following code some lines above:


    if ( oldViewport.pageNumber == viewport.pageNumber || !oldViewport.isValid() )
    {
        // if page is unchanged save the viewport at current position in queue
        oldViewport = viewport;
    }

So if the old viewport is not valid (on initial startup), the oldViewport it set to the current viewport -> we won't be able to determine a page change in that case, because oldViewport.pageNumber and d->m_viewportIterator.pageNumber will be the same.

Is the patch ok to commit otherwise?


- Tobias


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


On Aug. 21, 2012, 7:06 a.m., Tobias Koenig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106058/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2012, 7:06 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> The DocumentObserver API currently does not provide a way to notify about page changes, only about viewport changes. That means each implementation of DocumentObserver (e.g. TOC, MiniBar, SideReview etc.) kept a private variable to keep track of the current page to detect page changes.
> 
> This patch moves the page change detection into the Okular::Document class and extends the Okular::DocumentObserver API with the two callbacks notifyCurrentPageAboutToBeChanged(int page) and notifyCurrentPageChanged(int page).
> 
> That allows the implementations of Okular::DocumentObserver to just reimplement the notifyCurrentPageChanged() callback instead of reimplementing the page-changed-detection logic.
> 
> Since the two callbacks are always invoked on _all_ listeners, the PageView has now a chance to get informed about page changes even though it's notifyViewport() method is not invoked if it changes the viewport itself.
> 
> 
> Diffs
> -----
> 
>   core/document.cpp f6bf699 
>   core/document_p.h 54e922d 
>   core/observer.h 76c096c 
>   core/observer.cpp 0201a1d 
>   ui/minibar.h acb1163 
>   ui/minibar.cpp 051df72 
>   ui/pagesizelabel.h ea508b8 
>   ui/pagesizelabel.cpp 4a80779 
>   ui/pageview.h 43ca2ab 
>   ui/pageview.cpp 5e04412 
>   ui/presentationwidget.h 20dbcbb 
>   ui/presentationwidget.cpp f4da539 
>   ui/side_reviews.h d063b7b 
>   ui/side_reviews.cpp a036c48 
>   ui/thumbnaillist.h 0d7136b 
>   ui/thumbnaillist.cpp 8b23025 
>   ui/toc.h 4e63ef6 
>   ui/toc.cpp 3203c79 
> 
> Diff: http://git.reviewboard.kde.org/r/106058/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tobias Koenig
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20120829/6d45dde5/attachment.html>