On 10/30/2010 01:30 AM, Albert Astals Cid wrote:
>> because I don't really know what the desired behaviour is when
>> "!useMediaBox&&  !crop" is passed.
>
> I think false, true and false, false actually create a very similar if nto the
> same behaviour.
>
> But that leads me to the fact that (even if noone that we know was using it),
> your current patch makes it impossible to get the same behaviour of having
> useMediaBox = true and crop = true. Since that gave you a page as big as the
> mediabox but the contents cropped to the cropbox, so i really think the
> display methods should have two parameters
>
> Page::PageBox box, Page::PageBox cropBox
>
> that mimic exactly what the Gfx constructor parameters do, and we'd probably
> need another value for the PageBox enum that would be something like NoBox so
> you can pass it to boxToCrop if you don't want any cropping (passing a null to
> Gfx).
>
> What do you think?

Funny, I thought the same thing today. The way it is now really doesn't 
make any sense to me. With my patch as it is, the better description 
would probably be "clip painting to pagebox".

I don't think we need a noBox value. When not specifying a crop-value, 
we use the box for the pagesize as cropbox (so no paint-cropping 
occurs). If no parameters are specified, we just use MediaBox for both. 
Of course, it wouldn't make any sense to use e.g. trimbox for pagesize 
and mediabox for cropping, but we could trim the cropping-box to the 
pagesize box before passing it on. Good?

Would it be ok for you to commit the patch as it is now, so I can work 
from there instead of fighting with git, or would you prefer one patch 
trying to get it right?

>> and modify callers accordingly. Do you agree?
>
> Please don't do any change to existing code unless it's totally necessary.

Oh. I was thinking about doing some more changes that would - in my eyes 
- simplify the code (e.g. not passing null to Gfx c'tor, but passing 
mediaBox instead). Are you worried about regressions, or whats the idea 
behind this?

 >> P.S: Sorry for making this a two-patch-mess, I need to learn git.
 >
 > That makes two of us ;-.)

Then how do you send patches to poppler? For now, I'd be interested to 
find a way around learning git :|

cheers,
ben
_______________________________________________
poppler mailing list
poppler@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/poppler