Review Request 113704: Fix EPS plugin

[Alex Merry]

> On Nov. 10, 2013, 11:20 a.m., David Faure wrote:
> > tier1/kguiaddons/src/plugins/imageformats/eps.cpp, line 300
> > <http://git.reviewboard.kde.org/r/113704/diff/1/?file=211396#file211396line300>
> >
> >     isn't there a better method to call than pid()?

You're right; I missed the state() method last time I read through the QProcess API.


> On Nov. 10, 2013, 11:20 a.m., David Faure wrote:
> > tier1/kguiaddons/src/plugins/imageformats/eps.cpp, line 216
> > <http://git.reviewboard.kde.org/r/113704/diff/1/?file=211396#file211396line216>
> >
> >     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 ;)

Now reads in 4K blocks.


> On Nov. 10, 2013, 11:20 a.m., David Faure wrote:
> > tier1/kguiaddons/src/plugins/imageformats/eps.cpp, line 53
> > <http://git.reviewboard.kde.org/r/113704/diff/1/?file=211396#file211396line53>
> >
> >     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.

Can't do it for the unsigned things, but I've changed it for the others.


- Alex


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


On Nov. 10, 2013, 6:16 p.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113704/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2013, 6:16 p.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/294b19f7/attachment.html>