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

Out of order defs/references #102

Closed
dnfield opened this issue Jan 14, 2019 · 31 comments · Fixed by #782
Closed

Out of order defs/references #102

dnfield opened this issue Jan 14, 2019 · 31 comments · Fixed by #782

Comments

@dnfield
Copy link
Owner

dnfield commented Jan 14, 2019

Right now, an error is reported if you try to use a file that does something like the following:

<svg>
  <path fill="url(#MyGradient)" d="..." />
  <linearGradient id="MyGradient" ...>
    ...
  </linearGradient>
</svg>

This is done to make sure we don't have to maintain a full DOM or traverse the XML more than once.

However, it should be possible to keep an in memory structure of defs (probably a Map of some sort) we expect to see but haven't seen yet, and go back and back-fill them (and then remove them from the structure after we found the def they're looking for) with only a little bit of memory overhead.

See also #68 and #97

@IchordeDionysos
Copy link
Contributor

Would really appreciate if this would be implemented.
We are having way to many/dynamic SVGs to change them all :)

@NotThatBowser
Copy link

Would be useful. Affinity Designer produces SVGs with defs at the bottom, for example.

@dnfield
Copy link
Owner Author

dnfield commented Oct 21, 2019

I thought I had a way to do this, but all the ways I've been able to come up with are pretty inefficient.

If we defer resolution until drawing time (e.g. a lot of things that are now static properties become functions/getters that may not work during the parse phase), we can avoid reparsing the document but may get into strange situations where we have lots of indirection going on via functions. For example, getting a Drawable's fill might actually be a function that calls a function that calls a function that returns the data by calling a few other functions to resolve its properties.

If, on the other hand, we always parse defs in any order, we have to go to a DOM based model of parsing and do a lot of jumping around the document/tree, which will be very inefficient on particularly large SVGs.

This should be something that could be preprocessed away and avoid these complexities/inefficiencies.

@hash404
Copy link

hash404 commented Oct 22, 2019

I wonder if you there's a way to add build time pre-processing to avoid inefficiencies.

Something like how built_value.dart (https://github.com/google/built_value.dart) generates Dart files, could you have flutter_svg generating a compressed SVG that matches the format required by the package?

@dnfield
Copy link
Owner Author

dnfield commented Oct 22, 2019

That's definitely doable, and something I've considered exploring but haven't had time to prioritize - or evidence that it would really bump performance enough to be worth it. And unfortunately it wouldn't help with network based images

I suppose a middle road might be having something like that and then allowing for a less efficient parsing algorithm - which would make precompiled images a bit faster but dynamic images even slower.

@dnfield
Copy link
Owner Author

dnfield commented Jan 28, 2020

I thought I had updated this again, but I guess not.

Supporting this introduces a lot of complexity. In particular, references can have other references, which can have other references, etc.

To fully support this would require some substantial reworking - the simplest cases of only one level of nesting aren't too bad, but when those nested references have other nested references, it becomes a big ugly list of closures everywhere to resolve things, and will be much less efficient both time and space wise, not to mention getitng a lot harder to reason about.

I'm really hesitant to support this. But I also understand it's frustrating when you have an SVG you don't control that just comes this way.

@ngaurav
Copy link

ngaurav commented Jan 29, 2020

Why not just provisionally add support for single level nesting? Once done we can get a better idea if we need multi-level nesting support. Maybe multi-level nesting is very rare compared to single-level nesting.

@ngaurav
Copy link

ngaurav commented Jan 29, 2020

On second thought, If a file only has single-level nesting then it can programatically preprocessed to rearrange out-of-order declarations.

@IchordeDionysos
Copy link
Contributor

And if you can't? @ngaurav
Stored on another server/too many of them.

@dnfield
Copy link
Owner Author

dnfield commented Jan 29, 2020

@IchordeDionysos - you could preprocess the SVG from the server before you pass it to this library.

In fact, maybe that's the right way to handle this - write a preprocessing package to rearrange defs. That way people who want to pay the performance and complexity costs (or have no choice) can, without forcing them on people with more sane SVGs.

@willhaslett
Copy link

What's the status on this? I have my defs up top right after <svg>, but am still getting this error.

@cihadturhan
Copy link

FYI, figma exports definitions after so SVGs are not rendered.
Figma has a larger user base than affinity and i think we'll hear more complains on this.

@agueroveraalvaro
Copy link

is there a solution please? :)

@cesc1802
Copy link

i facing with this issue and figma design tool :( anyone can help me?

@hpbl
Copy link

hpbl commented Jul 6, 2021

@willhaslett did you find a solution?

@OlegNovosad
Copy link

The workaround for me was to move the <defs> up, as the first children in the <svg> element. E.g.:

Before:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" viewBox="0 0 20 20">
    <path fill="url(#paint0_linear)" d="M10 20c5.523 0 10-4.477 10-10S15.523 0 10 0 0 4.477 0 10s4.477 10 10 10z"/>
    <path fill="#fff" fill-rule="evenodd" d="M4.527 9.894c2.915-1.27 4.859-2.107 5.832-2.512 2.777-1.155 3.354-1.356 3.73-1.362.083-.002.268.019.387.116.102.082.13.193.143.27.013.079.03.256.016.394-.15 1.582-.801 5.419-1.133 7.19-.14.75-.416 1-.683 1.025-.58.053-1.022-.384-1.584-.752-.88-.577-1.377-.937-2.232-1.5-.987-.65-.347-1.008.216-1.592.147-.153 2.706-2.48 2.755-2.691.006-.027.012-.125-.046-.177-.059-.052-.145-.034-.208-.02-.088.02-1.494.949-4.218 2.788-.399.274-.76.407-1.084.4-.357-.008-1.044-.202-1.554-.368-.627-.203-1.124-.31-1.081-.656.022-.18.27-.365.744-.553z" clip-rule="evenodd"/>
    <defs>
        <linearGradient id="paint0_linear" x1="10" x2="10" y1="0" y2="19.852" gradientUnits="userSpaceOnUse">
            <stop stop-color="#2AABEE"/>
            <stop offset="1" stop-color="#229ED9"/>
        </linearGradient>
    </defs>
</svg>

After:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" viewBox="0 0 20 20">
    <defs>
        <linearGradient id="paint0_linear" x1="10" x2="10" y1="0" y2="19.852" gradientUnits="userSpaceOnUse">
            <stop stop-color="#2AABEE"/>
            <stop offset="1" stop-color="#229ED9"/>
        </linearGradient>
    </defs>
    <path fill="url(#paint0_linear)" d="M10 20c5.523 0 10-4.477 10-10S15.523 0 10 0 0 4.477 0 10s4.477 10 10 10z"/>
    <path fill="#fff" fill-rule="evenodd" d="M4.527 9.894c2.915-1.27 4.859-2.107 5.832-2.512 2.777-1.155 3.354-1.356 3.73-1.362.083-.002.268.019.387.116.102.082.13.193.143.27.013.079.03.256.016.394-.15 1.582-.801 5.419-1.133 7.19-.14.75-.416 1-.683 1.025-.58.053-1.022-.384-1.584-.752-.88-.577-1.377-.937-2.232-1.5-.987-.65-.347-1.008.216-1.592.147-.153 2.706-2.48 2.755-2.691.006-.027.012-.125-.046-.177-.059-.052-.145-.034-.208-.02-.088.02-1.494.949-4.218 2.788-.399.274-.76.407-1.084.4-.357-.008-1.044-.202-1.554-.368-.627-.203-1.124-.31-1.081-.656.022-.18.27-.365.744-.553z" clip-rule="evenodd"/>
</svg>

@ebwood
Copy link

ebwood commented Aug 30, 2021

Any solution? Still not working.
tmdb.svg

@Tokenyet
Copy link

Tokenyet commented Oct 29, 2021

I make a quick program for reordering <defs> to the first child, hope It help someone. By the way, I still can't use the svg from designer exported from figma, even If I reorder the svgs.

use flutter_svg_opt as dev dependency, and run flutter pub run flutter_svg_opt:main (will change all svgs under assets by default).

@Vedsaga
Copy link

Vedsaga commented Dec 25, 2021

The workaround for me was to move the <defs> up, as the first children in the <svg> element. E.g.:

Before:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" viewBox="0 0 20 20">
    <path fill="url(#paint0_linear)" d="M10 20c5.523 0 10-4.477 10-10S15.523 0 10 0 0 4.477 0 10s4.477 10 10 10z"/>
    <path fill="#fff" fill-rule="evenodd" d="M4.527 9.894c2.915-1.27 4.859-2.107 5.832-2.512 2.777-1.155 3.354-1.356 3.73-1.362.083-.002.268.019.387.116.102.082.13.193.143.27.013.079.03.256.016.394-.15 1.582-.801 5.419-1.133 7.19-.14.75-.416 1-.683 1.025-.58.053-1.022-.384-1.584-.752-.88-.577-1.377-.937-2.232-1.5-.987-.65-.347-1.008.216-1.592.147-.153 2.706-2.48 2.755-2.691.006-.027.012-.125-.046-.177-.059-.052-.145-.034-.208-.02-.088.02-1.494.949-4.218 2.788-.399.274-.76.407-1.084.4-.357-.008-1.044-.202-1.554-.368-.627-.203-1.124-.31-1.081-.656.022-.18.27-.365.744-.553z" clip-rule="evenodd"/>
    <defs>
        <linearGradient id="paint0_linear" x1="10" x2="10" y1="0" y2="19.852" gradientUnits="userSpaceOnUse">
            <stop stop-color="#2AABEE"/>
            <stop offset="1" stop-color="#229ED9"/>
        </linearGradient>
    </defs>
</svg>

After:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" viewBox="0 0 20 20">
    <defs>
        <linearGradient id="paint0_linear" x1="10" x2="10" y1="0" y2="19.852" gradientUnits="userSpaceOnUse">
            <stop stop-color="#2AABEE"/>
            <stop offset="1" stop-color="#229ED9"/>
        </linearGradient>
    </defs>
    <path fill="url(#paint0_linear)" d="M10 20c5.523 0 10-4.477 10-10S15.523 0 10 0 0 4.477 0 10s4.477 10 10 10z"/>
    <path fill="#fff" fill-rule="evenodd" d="M4.527 9.894c2.915-1.27 4.859-2.107 5.832-2.512 2.777-1.155 3.354-1.356 3.73-1.362.083-.002.268.019.387.116.102.082.13.193.143.27.013.079.03.256.016.394-.15 1.582-.801 5.419-1.133 7.19-.14.75-.416 1-.683 1.025-.58.053-1.022-.384-1.584-.752-.88-.577-1.377-.937-2.232-1.5-.987-.65-.347-1.008.216-1.592.147-.153 2.706-2.48 2.755-2.691.006-.027.012-.125-.046-.177-.059-.052-.145-.034-.208-.02-.088.02-1.494.949-4.218 2.788-.399.274-.76.407-1.084.4-.357-.008-1.044-.202-1.554-.368-.627-.203-1.124-.31-1.081-.656.022-.18.27-.365.744-.553z" clip-rule="evenodd"/>
</svg>

Yaap, this actually works...

@LearningLeopard
Copy link

Is this error solved? The rearrangement of tags didn't solve the issue

@Vedsaga
Copy link

Vedsaga commented Jan 23, 2022

Is this error solved? The rearrangement of tags didn't solve the issue

Ohh for me it did solved... If you put your <path > tag at the bottom

@LearningLeopard
Copy link

My svg doesn't have a <path> tag but it has <rect> tag

@Vedsaga
Copy link

Vedsaga commented Jan 23, 2022

My svg doesn't have a <path> tag but it has <rect> tag

Ohh I haven't worked with <rect> 😅 my be try finding svg which have path in that... Or if possible you can share one of your svg code here I will try see if I can convert it into path like format

@LearningLeopard
Copy link

LearningLeopard commented Jan 23, 2022

headphone_on_book

Here is the svg file. You can download it and open it in a code editor since pasting the code of svg seems pretty clumsy here

@Vedsaga
Copy link

Vedsaga commented Jan 23, 2022

headphone_on_book

this does seems to be svg... this seems to be png image I am not sure if this can be scan be supported.. so, I would suggest to simply use png instead of converting png to svg

@sahharYoucef
Copy link

any updates on this issue ?

@dnfield
Copy link
Owner Author

dnfield commented May 31, 2022

I'm working on a new package that supports out of order defs. However, as I expected, doing so resulted in some noticable performance regressions. I'm not sure it'll make sense to ever support in this package, which is already slow enough atparsing the SVGs.

@matejsnoha
Copy link

Thank you for your work. From my point of view it would be great if the library was able to render more SVGs, support for the really expensive features could be optional.

To work around this, we preprocess user-supplied SVGs where we move the defs to the beginning and we do a topological sort of the defs which solves the problem when they reference each other but were defined in the wrong order.

@MagnusJohansson
Copy link

I make a quick program for reordering <defs> to the first child, hope It help someone. By the way, I still can't use the svg from designer exported from figma, even If I reorder the svgs.

use flutter_svg_opt as dev dependency, and run flutter pub run flutter_svg_opt:main (will change all svgs under assets by default).

@Tokenyet would you please consider updating your package to use xml v6 ? Thanks.

@binSaed
Copy link

binSaed commented Dec 15, 2022

@ALL

  flutter_svg:
    git:
      url: https://github.com/binSaed/flutter_svg.git
      ref: fix_defs_order

@dmutoni
Copy link

dmutoni commented Mar 28, 2023

I'm working on a new package that supports out of order defs. However, as I expected, doing so resulted in some noticable performance regressions. I'm not sure it'll make sense to ever support in this package, which is already slow enough atparsing the SVGs.

Any updates?

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 a pull request may close this issue.