[poppler] [PATCH] initial clean poppler-qt implementation

[Albert Astals Cid]

A Divendres 01 Abril 2005 23:08, vàreu escriure:
> Albert Astals Cid wrote:
> > I've lost the original mail so i'm answering without any quoting, sorry
> >
> > in the next few lines we = kpdf
> >
> > A few things we have to say about this:
> >
> > ### PopplerDocument *PopplerDocument::load(const QString &filePath) ###
> >  - We can accept the current implementation, what we will do is check if
> > the result is null, if it is report there was an error opening the file,
> > if we got a document and is locked then ask for a password and try to
> > unlock it. It is something "similar" to what we do now.
>
> Is there another API that would work better with kpdf you would rather use?
No, we don't use exceptions nor any other default mechanism so this is as good 
as other

>
> > ### void PopplerPage::renderToPixmap(QPixmap **q) ###
> >  - How expensive is creating a new SplashOutputDev each time you call
> > renderToPixmap? Maybe it could be worth creating one at the begining and
> > reuse it
>
> I'm quite sure this isn't a concern, since all it's doing is allocating
> and initializing a couple of classes.  By a wide margin, the overhead in
> rendering a page is still in rasterization and compositing the contents.
> Of course, I haven't profiled this yet so I wrong.
I'll trade some memory for gaining some microseconds of speed every day of the 
week.

>
> >  - We have an option to draw with paperColor different than wait so
> > either we need a QColor parameter to renderToPixmap or a
> > setPaperColor(QColor c) operation
>
> Oh... hmm... is this really useful?  Do you have that option in kpdf?
> The PDF spec says that the drawing is expected to be composited against
> a white background.
One of the guys of kdeaccessibility suggested that changing background paper 
color may be good for people with visual deficiencies so we have it in the 
accessibility section of the config dialog (you know KDE is bloated and we 
have config options for everything ;-))

Albert

>
> > ### PDFDisplay class ###
> > This is for testing only, isn't?
>
> I would assume so, it's in the test-poppler-qt.cpp file ;-)
>
> cheers,
> Kristian