deeplook / svglib

Read SVG files and convert them to other formats.
GNU Lesser General Public License v3.0
307 stars 79 forks source link

Issue #366 - Add support for transforms on clipPath elements #367

Closed blayzen-w closed 1 year ago

blayzen-w commented 1 year ago

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.

blayzen-w commented 1 year ago

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:

<svg namespace="http://www.w3.org/XML/1998/namespace" 
     xmlns="http://www.w3.org/2000/svg" 
     xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" 
     viewBox="0 0 100 100">

    <defs>
        <clipPath id="clip-1">
            <path transform="scale(2,1)" d="M 50, 50 m -50, 0 a 50,50 0 1,0 100,0 a 50,50 0 1,0 -100,0"/>
        </clipPath>
    </defs>
    <g transform="scale(1,3)">
        <g clip-path="url(#clip-1)">
            <path d="M50,50H0V0h50V50z" fill="red"/>
        </g>
    </g>
</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.

blayzen-w commented 1 year ago

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 (if not (isinstance(node, Path) and node.isClipPath)).

    def drawNode(self, node):
        """This is the recursive method called for each node
        in the tree"""
        #print "pdf:drawNode", self
        #if node.__class__ is Wedge: stop
        if not (isinstance(node, Path) and node.isClipPath):
            self._canvas.saveState()

        #apply state changes
        deltas = getStateDelta(node)
        self._tracker.push(deltas)
        self.applyStateChanges(deltas, {})

        #draw the object, or recurse
        self.drawNodeDispatcher(node)

        self._tracker.pop()
        if not (isinstance(node, Path) and node.isClipPath):
            self._canvas.restoreState()

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.

claudep commented 1 year ago

You can submit patches to reportlab with attaching the patch to a message to the reportlab-users@lists2.reportlab.com 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

blayzen-w commented 1 year ago

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.

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 objects by enclosing the modification of the clipping path and the painting of those objects between a pair of q and Q operators. - page 138.

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.

claudep commented 1 year ago

Thanks a lot for your explorations. Feel free to close or to let open depending on your future plans.

blayzen-w commented 1 year ago

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.