[Okular-devel] Review Request 113973: Proof of concept magnifying glass found in old kdvi (or current xdvi)

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113973/#review50601
-----------------------------------------------------------


Looks reasonable and seems to work quite well, there's one issue though, which is that we only support tiled rendering for PDF files and for images, so if i open a DVI and try to use the magnifier it won't be of much use (it will either be pixelated or try to request a huge pixmap that won't be renderer). Maybe we should disable the magnifier if there's no tile support for the format [document->supportsTiles()]?


conf/okular.kcfg
<https://git.reviewboard.kde.org/r/113973/#comment35591>

    This needs to be at the end, otherwise i think you kill the kconfig way of saving/restoring Enum types (by number) since Magnifier is stealing RectSelect position.



ui/magnifierview.cpp
<https://git.reviewboard.kde.org/r/113973/#comment35594>

    This means we are zooming at 1000% right? It's a bit weird since i can say zoom up to 1600% in the page and then the magnifier view gives me less zoom than the one i have :D
    
    Not sure how to fix this thought or if it matters at all.



ui/pageview.cpp
<https://git.reviewboard.kde.org/r/113973/#comment35593>

    Please just add a new "int" here, makes it easier to read (same for the following groups of variables).
    
    Also some of these int seem to be const, can you please mark those that are const appropiately?



ui/pageview.cpp
<https://git.reviewboard.kde.org/r/113973/#comment35592>

    Please move the definition of y to the next line


- Albert Astals Cid


On Feb. 22, 2014, 6:01 p.m., Michal Humpula wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/113973/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2014, 6:01 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> I realy missed the magnifying glass feature from the old kdvi, which comes very handy if you are doing typesetting. So I finaly told myself to hack it in and here comes the patch for it.
> 
> The 01-hack.patch is allowing the document to render big pixmaps for non tiled views. The rest is quite straightforward.
> 
> The magnifier can scroll the view "apropriately", doesn't fall of the edges and hovers correctly from one page to another.
> 
> So the remaining question is, what is the status of this:
>  core/document_p.h:213  "// FIXME This is a hack, we need to support multiple tiled observers, but for the moment we only support one"
> Is someone working on it?
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 63d2922 
>   conf/okular.kcfg f954c9a 
>   part.rc 0b9cee5 
>   ui/magnifierview.h PRE-CREATION 
>   ui/magnifierview.cpp PRE-CREATION 
>   ui/pageview.h 9c15af6 
>   ui/pageview.cpp 7c7f785 
> 
> Diff: https://git.reviewboard.kde.org/r/113973/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michal Humpula
> 
>

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