meerk40t / svgelements

SVG Parsing for Elements, Paths, and other SVG Objects.
MIT License
127 stars 28 forks source link

[Bug]: Cannot handle post-defined elements & reused <use> elements #170

Closed YishiMichael closed 1 year ago

YishiMichael commented 2 years ago

Summary Description

There are some problems when parsing elements which refer to other elements defined later, as well as elements with a transform parameter which are reused for multiple times. The latter of them may have been mentioned in #169.

Additional Details

When trying to deal with https://github.com/3b1b/manim/issues/1760, I notice that svgelements fails to handle the following svg file:

<?xml version='1.0' encoding='UTF-8'?>
<!-- This file was generated by dvisvgm 2.11.1 -->
<svg version='1.1' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='30.033152pt' height='6.789661pt' viewBox='85.143591 -71.954169 30.033152 6.789661'>
<defs>
<path id='g4-65' d='M5.378384 0L5.427878-.239223L5.114414-.26397C4.858693-.288717 4.825697-.404204 4.784452-.742415L4.174022-5.708346H3.588339L2.202498-3.274875C1.781796-2.540709 1.097124-1.3116 .791909-.816656C.52794-.387706 .387706-.296966 .131985-.272219L-.140234-.239223L-.189728 0H1.666309L1.715803-.239223L1.262105-.280468C1.097124-.296966 1.080626-.412453 1.154868-.585683C1.427087-1.113622 1.699305-1.649811 2.00452-2.202498H3.852309L4.042037-.602181C4.066784-.362958 4.000792-.296966 3.835811-.280468L3.398611-.239223L3.349116 0H5.378384ZM3.811063-2.515962H2.169501C2.606701-3.332618 3.060399-4.141026 3.505848-4.941184H3.522346L3.811063-2.515962Z'/>
<use id='g6-65' xlink:href='#g3-65' transform='scale(1.166668)'/>
<path id='g3-65' d='M5.361886 0V-.239223C4.883441-.280468 4.792701-.305215 4.644218-.734166L2.895418-5.708346H2.284988L1.418837-3.266626C1.163117-2.548958 .816656-1.559071 .52794-.808407C.354709-.362958 .280468-.26397-.239223-.239223V0H1.57557V-.239223L1.146619-.280468C.899147-.305215 .8744-.387706 .940392-.61043C1.080626-1.105373 1.253856-1.616815 1.443585-2.202498H3.291373L3.84406-.626928C3.92655-.387706 3.885305-.296966 3.621335-.272219L3.250128-.239223V0H5.361886ZM3.192384-2.515962H1.550822C1.814792-3.340867 2.103509-4.149275 2.350981-4.875191H2.375728L3.192384-2.515962Z'/>
</defs>
<g id='page1'>
<use x='85.143591' y='-65.164508' xlink:href='#g4-65'/>
<use x='90.669586' y='-65.164508' xlink:href='#g6-65'/>
<use x='96.801578' y='-65.164508' xlink:href='#g6-65'/>
<use x='102.93357' y='-65.164508' xlink:href='#g6-65'/>
<use x='109.065562' y='-65.164508' xlink:href='#g6-65'/>
</g>
</svg>

There're two issues popping out. Firstly, the id g6-65 uses glyph g3-65, which is defined after it, making svgelements cannot recognize the element and returns SVGElement instances instead of Paths. Secondly, if I switch these two lines, from the result of parsing it can be noticed that the g6-65 elements are really far apart, and are even not aligned horizontally given the y attributes are equal. The latter of them may have been mentioned in #169.

tatarize commented 2 years ago

This has been noticed before. If the use is defined in a defs at the time everything is good. But, the would-be look ahead parts where the use in defs uses the thing not yet defined is a problem. Specifically #160 is intended to address this based on #156. The only thing to do is pre-parse the tree and have access to the entire shadow-dom tree before attaching the use elements.

It's on the agenda but is a bit of a rarer use case. It does happen and use objects referencing future objects just like css sheets affecting objects in the past, isn't quite there because of the ordering of the parsing.

tatarize commented 1 year ago

This was fixed in #160.

Specifically it should handle your case. Even when that ordering worked it would get the element with the get by id and have used the same use id. Now it should play back the relevant portions of the xml within a use object correctly.

YishiMichael commented 1 year ago

Many thanks for your help! I tried with the svg above and found all use elements are mapped to their corresponding elements. However, I noticed that the transform attributes of these elements are wierdly identity matrices, making these shapes completely overlapping with each other, rather than arranged horizontally as the x and y attributes suggest. Parsing result: image

Expected result: image

I wonder if I've missed any method to properly obtain transformed positions, or any bug occurs somewhere.

tatarize commented 1 year ago

It appears that I missed a spot. The x, y values are supposed to override the other values. So if you set x, y values, it's supposed to compound them in a particular way that is currently wrong. I'll check into correcting that.

YishiMichael commented 1 year ago

When briefly scanning the code, I came up with two little suggestions.

  1. There's a reference for variable pi in function RoundShape._ramanujan_length, but is not imported definitely. It should be changed to tau / 2.
  2. I guess maybe Use class can inherit Group directly, as Use is itself a container. This would also help when using SVG.elements method to expand elements contained inside Use instances.
tatarize commented 1 year ago
  1. Good catch. I only provisionally imported pi to provide tau for versions py<=3.5
  2. I tend to mimic the DOM classes since it gives me fewer problems. https://www.w3.org/TR/SVG11/struct.html#UseElement https://www.w3.org/TR/SVG11/struct.html#Groups It looks like use is not a container element or subclass of group. Though both are structural elements.

I did expand the Use instances a bit to include the x, y, height, and width specifically because I needed those to not propagate per spec.

This should be fixed in https://github.com/meerk40t/svgelements/pull/193

image

tatarize commented 1 year ago

Okay, should be fixed in 1.8.1

YishiMichael commented 1 year ago

Thanks a lot! Now the result is as expected.