[Okular-devel] Review Request 111410: Selection tool: copy/extract as vector graphic by calling "pdftocairo"

[Albert Astals Cid]

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


So yes, this is going in the right direction, there's lots of things to do (actually you have a todo). Some comments below


core/document.h
<http://git.reviewboard.kde.org/r/111410/#comment27989>

    Can you add since markers to the new functions in core/ public header files? New okular version will be 0.18



core/generator.h
<http://git.reviewboard.kde.org/r/111410/#comment27990>

    @since markers here too



core/generator.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27991>

    Is it Joint?



generators/poppler/generator_pdf.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27992>

    Not sure calling this before main and the other KComponent stuff is created is a good idea, i'd prefer if you fill it in the constructor



generators/poppler/generator_pdf.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27993>

    Something's wrong with the indentation



generators/poppler/generator_pdf.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27994>

    You don't need pdftocairo to export to pdf or ps, poppler knows how to do that.



generators/poppler/generator_pdf.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27996>

    This is interesting, since pdftocairo not always support svg, if you look at the code it is surrounded by 
    #if CAIRO_HAS_SVG_SURFACE
    
    Maybe you should grep pdftocairo -h (eeeek) output to make sure it supports svg?



generators/poppler/generator_pdf.cpp
<http://git.reviewboard.kde.org/r/111410/#comment27995>

    indentation is wrong here again


- Albert Astals Cid


On July 23, 2013, 9:26 p.m., Thomas Fischer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111410/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 9:26 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> This patch implements the feature request of bug 321350: if a PDF file is viewed, the selection tool offers the new extraction method "vector" which allows to save to a file (PDF, SVG, EPS, PostScript). The crop operation is performed by calling "pdftocairo" with matching arguments. The resulting file contains the original PDF file's content without rendering it to a pixmap.
> 
> I am not sure if calling an external program is an acceptable solution for this problem. However, it is tested if the program is available before showing the new option. Alternatively, the code of pdftocairo (as part of poppler) would had to be copied and integrated into Okular increasing the solution's complexity. I am not aware of a similar solution available using poppler-qt4 only. Maybe using a QPrinter printing to PDF would have been an alternative, but again this seemed to be too complex.
> 
> 
> Diffs
> -----
> 
>   core/document.h fe296e0 
>   core/document.cpp 6731d87 
>   core/generator.h a5a971b 
>   core/generator.cpp 41beb92 
>   generators/poppler/generator_pdf.h 5d5853a 
>   generators/poppler/generator_pdf.cpp 122ece5 
>   ui/pageview.cpp 16b00ab 
> 
> Diff: http://git.reviewboard.kde.org/r/111410/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Thomas Fischer
> 
>

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