[poppler] Clipping text in Splash before rendering it

[Albert Astals Cid]

A Diumenge 14 Octubre 2007, Jeff Muizelaar va escriure:
> On Sun, Oct 14, 2007 at 01:17:05AM +0200, Albert Astals Cid wrote:
> > Hi, this patch has two parts.
> >
> > First, it removes all exit(1) calls in favor or the best i could find,
> > that is, return NULL (and hope the caller can understand NULL as error)
> > in the gmem routines and the correct ignore in DCTStream.
>
> What's the motivation for this? Won't the callers just segfault
> afterwards anyways? It looks like most callers assume that gammloc
> succeeds. If we wanted to deal with the out of memory situation it would
> require quite a bit of work to add all of the proper error handling
> code. At least with exisiting implementation the application dies
> cleanly instead of segfaulting. It would be nice to deal with
> out-of-memory properly, but I think it would be quite a bit of work...

The motivation is that without it my splash patch is worthless because the 
huge font tries to create a huge cache (before i can discard it because it's 
out of the clip area) that integer wraps a gmallocn call and i got an app 
exit(), making it return NULL gets it to work without any problem. 

I agree in most other situations will probably segfault, but as i said exit() 
in library code is a bad idea, so when we hit those segfaults because of the 
return NULL we try to fix them instead of the exit(1). 

Of course i can catch the integer wrapping in the code that calls gmallocn but 
why duplicating checks everywhere if you can have them at one place only?

Albert

>
> > Second, in Splash code it moves the resposability for checking if a glyph
> > is inside the clip rect from Splash::fillGlyph2 to its callers,
> > Splash::fillGlyph and Splash::fillChar, that way i can check inside
> > SplashFTFont::makeGlyph if the font is inside the clip rect before
> > rendering.
> >
> > This fixes crash due to memory exhaustion on KDE bug 150693 [1] because
> > that pdf specifies a HUGE font outside the clip rect :-/
>
> Didn't look at the splash stuff.
>
> > CairoOutputDev does not need to do that because cairo already does it.
> >
> > If noone objects to the patch (that is a bit intrusive but clean enough
> > imho) i will commit it to HEAD and 0.6 branch next friday.
>
> Regardless of whether this patch goes in or not, make sure you do it in
> two commits because the changes are completely unrelated.
>
> -Jeff
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler