[poppler] Poppler 0.12 release schedule

[Albert Astals Cid]

A Dijous, 14 de maig de 2009, Pino Toscano va escriure:
> 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.

If we are going to pass void * just pass void *, it's as ugly as a Qt::HANDLE 
and shows what you're doing more clearly imho.

> 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.

Agreed.

>
> > 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.

Yeah, but i don't really think that library belongs into the poppler scope, so 
let's just use that code until someone creates that lib :D

> >
> > 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?

In my opinion it's not needed at all, should be automatic on creation and on 
calls that need to change something (like i guess setDocumentProfileName)

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

About naming, should setDocumentProfileName be setOutputProfileName? I mean 
should i set there the profile of my monitor (output) or of the document 
(input)

Albert

>
> -) 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.