-
Notifications
You must be signed in to change notification settings - Fork 78
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
Issue #366 - Add support for transforms on clipPath elements #367
Issue #366 - Add support for transforms on clipPath elements #367
Conversation
The only issue that I have found so far is that scale transforms don't seem to be sandboxed correctly (translate works correctly). At this point I'm not sure if this is an issue with reportlab or svglib. I'll take a look at this some more on Monday but I'll put my findings below. Take the following svg:
The square should be 50x150, however its being generated at 100x150. It almost seems that the clip path scale it being applied to the square that its clipping. |
tests/test_samples.py
Outdated
f.seek(0) | ||
return f | ||
|
||
tar_data = opener(join(TEST_ROOT, "samples", archive_path)).read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I ask you to put this part in a separate PR, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may work on this now, in fact, wait a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #368. You can drop this part of the patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, no problem. I just wanted to make sure the test cases were passing.
It looks like that issue that I'm running into is a bug with reportlab. From what I can tell, the transformation is being applied when rendering the clipPath element, however when it's finished the state is never unapplied. So the transformation is then applied to the next element that is rendered. This can be seen in the drawNode function where it doesn't save and restore the canvas state if it's a path with clipPath set (
When the check is removed and the canvas state is saved and restored, the element is sized correctly but the clip path is not applied. They don't seem to host their code publicly so hopefully they are receptive to changes/bug reports from the public. Otherwise maybe we might be able to monkey patch it but I don't care for those too much. |
You can submit patches to reportlab with attaching the patch to a message to the [email protected] mailing list. The official repo is https://hg.reportlab.com/hg-public/reportlab, and you can find a Git mirror on https://github.com/MrBitBucket/reportlab-mirror |
@@ -895,7 +909,7 @@ | |||
class Svg2RlgShapeConverter(SvgShapeConverter): | |||
"""Converter from SVG shapes to RLG (ReportLab Graphics) shapes.""" | |||
|
|||
def convertShape(self, name, node, clipping=None): | |||
def convertShape(self, name, node, clipping=None, transform_shape=False): |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns
I haven't heard anything from reportlab so I did a bit of digging in the PDF ISO 32000-1 spec and found this interesting note.
So if I'm understanding this correctly, trying to apply a transform on the clipping path may be a dead end, since the clipping path transform needs to not depend on the current graphical state because it's referenced. I have almost no experience working with pdfs so I could be misunderstanding things, and I don't have access to ISO 32000-2 to see if anything has changed since 2008. I was hoping this would be an easy fix since Illustrator seems to import svgs just fine, but I guess not. I'm not sure when I will have time to continue working on this. If you want I can close the draft and open a new one once I find a better solution. |
Thanks a lot for your explorations. Feel free to close or to let open depending on your future plans. |
No problem. Thanks for all of your work on this project, I really appreciate it. I'll close this draft but leave the issue open so other people can reference it later. I hope to be able to work on it again sometime later this year but we have a few larger projects that need to get done first. |
See Issue #366 for details.
Let me know if you would like anything changed.
I want to validate this on a few more larger files before I move it out of draft mode.