Review Request 113704: Fix EPS plugin

[David Faure]

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



tier1/kguiaddons/src/plugins/imageformats/eps.cpp
<http://git.reviewboard.kde.org/r/113704/#comment31212>

    Prefer constructor-syntax over C casts.
    
    i.e.  qint64(value) rather than (qint64)(value).
    
    C casts (in general, not in this particular instance) can lead to a lot of trouble, I like code to compile with -Wold-style-cast.



tier1/kguiaddons/src/plugins/imageformats/eps.cpp
<http://git.reviewboard.kde.org/r/113704/#comment31214>

    You can even make that a warning - I don't think it's every supposed to happen.



tier1/kguiaddons/src/plugins/imageformats/eps.cpp
<http://git.reviewboard.kde.org/r/113704/#comment31215>

    Thank you, this is much safer than popen indeed...



tier1/kguiaddons/src/plugins/imageformats/eps.cpp
<http://git.reviewboard.kde.org/r/113704/#comment31216>

    This could be improved to avoid loading the whole image in memory, given that QImageReader can read incrementally from a QIODevice.
    
    (but I won't block this commit for that, it was there before ;)



tier1/kguiaddons/src/plugins/imageformats/eps.cpp
<http://git.reviewboard.kde.org/r/113704/#comment31218>

    isn't there a better method to call than pid()?


- David Faure


On Nov. 7, 2013, 11:32 a.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113704/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2013, 11:32 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> 4 commits.  Previously, writing support was completely broken (it would write a PDF file instead of an EPS file).  The rest of the changes mostly make it more maintainable.
> 
> 
> Disable EPS plugin on non-UNIX systems
> 
> It depends on the gs command-line utility being in PATH, which is
> unlikely on Windows, for example.
> 
> 
> Use QProcess to run gs when reading EPS images
> 
> 
> Fix writing EPS files
> 
> QPrinter is no longer capable of producing PostScript files, so we
> output to PDF then used GhostScript (or pdftops from Poppler) to convert
> the result to EPS.  Note that GhostScript is already used by the reading
> code.
> 
> 
> Use qCDebug for the EPS plugin
> 
> 
> Diffs
> -----
> 
>   tier1/kguiaddons/src/plugins/imageformats/CMakeLists.txt 005859ac6fd792f2589339bc68437dd2d965cc8f 
>   tier1/kguiaddons/src/plugins/imageformats/eps.cpp cb25052a9d7a01ea7a8ed8014726b762e8a5da16 
> 
> Diff: http://git.reviewboard.kde.org/r/113704/diff/
> 
> 
> Testing
> -------
> 
> Successfully converted a JPG file to EPS and back again; both the final JPG and the intermediate EPS file display properly with Gwenview/Okular from KDE SC 4.  This works both with pdftops present in PATH and without it (providing gs is in PATH).
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131110/ddd83ee9/attachment-0001.html>