Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clipping in PDF backend #1446

Closed

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Nov 1, 2012

This fixes a bug introduced in #1444, where the clipping information would be lost when using a path collection. I think this fixes things once and for all, but if not it's probably safest to just revert #1444 which was solving a much less serious bug than this one.

@mdboom
Copy link
Member Author

mdboom commented Nov 1, 2012

Ack. This is screwing around with text. Still working...

@@ -1535,8 +1535,6 @@ def draw_path_collection(self, gc, master_transform, paths, all_transforms,
path_codes.append(name)

output = self.file.output
gc = copy.copy(self.gc)
output(Op.gsave)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, getting into what this actually does isn't easy. gsave is q and grestore is Q, this corresponds to the PDF specification (http://www.adobe.com/devnet/pdf/pdf_reference.html section 4.4):

There is no way to enlarge the current clipping path or to set a new clipping path
without reference to the current one. However, since the clipping path is part of
the graphics state, its effect can be localized to specific graphics objects by en-
closing the modification of the clipping path and the painting of those objects
between a pair of q and Q operators (see Section 4.3.1, “Graphics State Stack”).
Execution of the Q operator causes the clipping path to revert to the value that
was saved by the q operator, before the clipping path was modified.

So I wonder what effect removing this local clipping path functionality has? @mdboom : would you mind providing some extra detail?

@pelson
Copy link
Member

pelson commented Nov 2, 2012

@jkseppan : Jouni, when @mdboom has finalised this PR, would you mind reviewing? I don't feel that I can provide particularly helpful feedback on this review.

In the meantime @mdboom: I think we should revert #1444, which will ease the pressure to get this PR merged. What do you think?

Thanks,

@mdboom
Copy link
Member Author

mdboom commented Nov 2, 2012

Yes -- I've gone ahead and reverted #1444. I thought this was a simple, obvious bug, but that turned out to be wrong.

@mdboom
Copy link
Member Author

mdboom commented Nov 8, 2012

Closing because it is fixed by #1450.

@mdboom mdboom closed this Nov 8, 2012
@mdboom mdboom deleted the fix_pdf_path_collection_state branch November 10, 2015 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants