[poppler] Re: [PATCH] poppler-qt links

[Albert Astals Cid]

A Dijous 21 Setembre 2006 00:53, Wilfried Huss va escriure:
> Am Wednesday, 20. September 2006 23:14 schrieb Albert Astals Cid:
> > A Dijous 07 Setembre 2006 23:10, Wilfried Huss va escriure:
> > > Am Thursday, 7. September 2006 11:48 schrieb Brad Hards:
> > > > On Thursday 07 September 2006 19:07, Wilfried Huss wrote:
> > > > > Hi,
> > > > >
> > > > > attached are two patches (for the qt3 and qt4 bindings) which
> > > > > change Page::links() in a way to make it work correctly even if the
> > > > > render function was not called with doLinks=true previously. The
> > > > > current code results in a crash in such a situation.
> > > >
> > > > I'm not sure I fully understand this code, but it looks like you are
> > > > recalculating the links every time.
> > > >
> > > > Have you timed this?
> > >
> > > No I havn't. With the current code, if you want to access link
> > > information, you also need to call renderToImage() first, which is
> > > definitely not a cheap operation.
> >
> > It is if you want to show the page ;-)
> >
> > > > Is it worth trying to cache it?
> > >
> > > One could try to remember if the last time renderToImage() had been
> > > called with doLinks=true, and then use the cached version of the link
> > > data structure. But I'm not sure if it is really worth it.
> >
> > Could you please time the diferences in
> > render + getlinks that are there already
> > and
> > render + getlinks using your code
> > .
> >
> > I agree that making the library more functional using your patch may be a
> > good thing, but there is no necessity in making it slower for the rest,
> > no?
> >
> > BTW if you don't have time for it, tellme and i'll test it next week at
> > aKademy.
>
> Probably the easiest thing, would be to just use a flag in Poppler::Page to
> remember if the last call to render had doLinks=true. If yes use the cached
> Links like we do now, and if not create a link structure like in the new
> code.

Ok, i profiled a bit and the time spent reasking the links after you already 
had them because you drawed the image was non trivial so i added the flag.

Do you agree with this patch?

Albert

>
> Greetings,
> Wilfried.
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qt-links.patch
Type: text/x-diff
Size: 6423 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20061012/d38ada67/qt-links.bin