[poppler] Poppler 0.12 release schedule

[Pino Toscano]

Alle martedì 12 maggio 2009, Hal V. Engel ha scritto:
> Here it is.  This patch applies and builds on git head as of this
> afternoon.

Nice!
Just as a general comment: there are many whitespace-only changes (like a tab 
that becomes four spaces); if you could avoid them, and produce a patch in 
unified format (`git diff -U` should be enough), it would be slightly more 
readable.

Now some comments about the Qt4 related parts:

> 1. I am not a cmake expert so I had to hack on qt4/src/CMakeLists.txt so
> that USE_CMS was defined for the qt4 stuff.  Someone with more cmake
> expertise almost for sure needs to fix my hack.

I don't like much the "have/don't-have LCMS" exposed in the public API. 
Especially that including lcms.h would break all the poppler-qt4 library 
users which don't have LCMS available.

The only solution I have in mind is the following: given the only LCMS bit 
exposed is cmsHPROFILE, which internally is a void*, it could be passed as 
Qt::HANDLE, like this:
    void setColorProfile(Qt::HANDLE cmsProfile);
    void setColorProfileName(const QString &name);
    Qt::HANDLE colorRGBProfile() const;
    Qt::HANDLE colorProfile() const;
and then doing 'cmsHPROFILE profile = (cmsHPROFILE)cmsProfile;' to cast the 
profile back and forth. Of course the apidox for the function would need to 
specify that the Qt::HANDLE refers to a cmsHPROFILE, and in case there's no 
color management that those handles are NULL.
Also, the function would always be compiled, and the actual implementations 
making nothing when HAVE_LCMS is not defined. For this, adding a
    static bool canManageColorProfiles();
would help users to know whether they can actually expect the function to do 
something.

> 4. Someone with Windows and/or Mac expertise needs to write versions of
> qt4/demos/viewer.cpp cmsHPROFILE PdfViewer::getProfileAtom () for those
> other OS's for this to work on Windows and on the Mac.   It might also be
> useful to move these functions to a support library so that they are
> available to anyone who uses the library.
>
> 5.  I am not a gkt programmer so I made no attempt to update any of the gtk
> stuff to support CM.  But the Qt4 code should provide enough clues for a
> gtk programmer to do the same basic stuff as I did with the Qt4 support
> library and the demo viewer.

Then the X11 specific code should be grouped inside #ifdef Q_WS_X11 blocks.

Other notes:

-) API: not sure whether it is necessary to expose 'int 
setupDocumentColorProfiles()', it seems more worth just making sure it is 
properly intialized when needed, either in the poppler core or when creating 
a new SplashOutputDev?

-) API: in Document::setDocumentProfileName(), QStringToGooString() creates a 
new GooString, which should be delete'd.

-) Qt4 demo: you don't need to create and hold a new QDesktopWidget, 
QApplication::desktop() will return the global one.

-) Qt4 demo: getProfileAtom() should be just colorProfile(), returning a 
Qt::HANDLE and being always defined.

-) Qt4 demo: the general coding style there is much like
http://qt.gitorious.org/qt/pages/QtCodingStyle, so please follow it, whenever 
possible.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090514/198c67ab/attachment.pgp