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

[Enhancement] Support SVG #15501

Closed
dnfield opened this issue Mar 13, 2018 · 34 comments
Closed

[Enhancement] Support SVG #15501

dnfield opened this issue Mar 13, 2018 · 34 comments

Comments

@dnfield
Copy link
Member

@dnfield dnfield commented Mar 13, 2018

EDIT:

https://pub.dev/packages/flutter_svg
https://github.com/dnfield/flutter_svg

If that's not meeting your needs or has a bug, please file bugs for it at https://github.com/dnfield/flutter_svg/issues/new


This is a split from #1831

While that issue can focus on a Flutter-specific vector drawable format, this issue should track SVG based support.

@cbazza @deborah-ufw were also particularly interested in this.

I have started some work here: https://github.com/dnfield/flutter_svg
I believe it will be beneficial to have some changes to engine as well, in particular:

  • Expose Skia's native SVG Path parsing logic
  • Expose SkPath::addCircle

I've started work on that here: https://github.com/dnfield/engine/tree/path_svg (branch currently contains other changes as well, will either split that out or submit it after a similar PR lands).

Some other thoughts from that thread:

  • Don't want/need full SVG support (scripting, foreign elements, animations (?), full CSS support, external references, other?)
  • Ideally should be async from top to bottom - including XML parsing
  • Want to be able to load assets created by designers at runtime for use on multiple devices/pixel densities
  • Should be useable anywhere an Image or Icon is today
@Hixie
Copy link
Member

@Hixie Hixie commented Mar 13, 2018

Please file individual bugs (or submit individual PRs) for the changes you want to make to the engine. There are some that I'm sure will be obviously beneficial and we should add them (e.g. addCircle). There are others that I'm concerned would have performance implications, or would be generally overly-specific, and which I think we should find better solutions for (e.g. parsing SVG path data). It would be good to have those conversations individually, though.

@dnfield
Copy link
Member Author

@dnfield dnfield commented Apr 26, 2018

My experimental SVG project no longer requires a specialized build of the engine. I stole Chromium's SVG parsing logic and ported it to dart here: https://github.com/dnfield/flutter_svg/blob/master/lib/src/parsers/path.dart - this takes care of not just parsing the paths, but normalizing them so that we can use the exposed calls for arcs (and helps handle smooth bezier/quad commands and h/v lineTo commands).

Anyone else who's interested in this issue is welcome to contribute. There'll be plenty to do, but this prototype can render a lot of simple icon type SVGs at this point.

@dnfield
Copy link
Member Author

@dnfield dnfield commented Apr 26, 2018

I think I'm going to split out that path parsing logic into it's own package. That package might have other useful path related routines like dashing or trimming paths.

@cbazza
Copy link

@cbazza cbazza commented Apr 26, 2018

Looks good but for the ArcTo path commands, you don't need to _decomposeArcToCubic, you can simply use path.arcToPoint.
https://docs.flutter.io/flutter/dart-ui/Path/arcToPoint.html

@dnfield
Copy link
Member Author

@dnfield dnfield commented Apr 26, 2018

Package here:

https://github.com/dnfield/flutter_path_drawing

https://pub.dartlang.org/packages/path_drawing

The new package fixes another bug, and adds a bunch of test paths from the Chromium tests.

I'll be removing the path parsing logic from flutter_svg in my next push and consuming this package instead.

@cbazza - Flutter only exposes a version of arcTo that would otherwise require calculating/transforming the values. _decomposeArcToCubic takes care of that (and also transforms the arc into a cubic). I'm not entirely sure, but I'm going to guess that the reason Chromium does this is performance related as well - they certainly could have made of use of other Skia arcTo overloads that would be easier, but they use this method instead. My assumption is that if the arcTo methods were really more performant/preferable, they would have just used those. Part of why I say that is that I'm assuming some similar decision was made by the Flutter team as to which overload of arcTo they would expose.

@cbazza
Copy link

@cbazza cbazza commented Apr 26, 2018

@dnfield
Flutter exposes 2 versions of arcTo and I think the second one arcToPoint has the exact parameters that matches what comes from SVG so it won't require transforming arc into cubics. Perhaps it is a performance choice though as you said.

@dnfield
Copy link
Member Author

@dnfield dnfield commented Apr 26, 2018

Wow, thanks for pointing that out. You got me curious though - I looked at the Skia implementation and it's actually pretty much the same Chromium code for this particular call to arcTo (translated to use Skia primitives/types/etc) 😆

So now this code exists in a bunch of places. I'm a little torn on what to do here - I could certainly remove it and just rely on the Skia implementation doing the same thing. I'll test it out, maybe try to benchmark it if I can - but I suspect the Skia version would only slightly edge out the Dart version.

@slightfoot
Copy link
Member

@slightfoot slightfoot commented Apr 27, 2018

@dnfield I have experiencing writing a SVG to Android Path lib. I'll join in on your escapades.

@csga5000
Copy link

@csga5000 csga5000 commented Sep 26, 2018

@Hixie If Skia has logic for parsing SVG path data, then why would we not want to support this? Surely that's performant enough. This is something supported on android and can kind of be done on iOS using PDF's though xcode parses the PDF to a bitmap at build time.

This provides several advantages where SVG graphics are appropriate including better support for screens of any resolution and significantly smaller APK's and IPA's as you don't have to deploy bitmaps (of which a single one is larger than most things that could appropriately be a SVG - and you have to deploy numerous versions of a bitmap). It also is convenient in circumstances where you want a scaling icon or different uses of an icon at different sizes.

I make a practice of using SVG's whenever I can in both web and mobile apps, and flutter being a modern framework I would expect to support this without me having to rely on a third party package. In the mean time I'll try @dnfield's package.

@dnfield
Copy link
Member Author

@dnfield dnfield commented Sep 26, 2018

@csga5000, in testing the Skia native logic for parsing path data isn't really any faster than using Dart code over thousands of iterations. It also adds a bit more code to the engine, which we're trying to keep small.

In addition to the svg library, I've also written a library to generate dart:ui Path objects at compile time from SVG paths, and I'd like to eventually look at extending that to more of the SVG spec.

@csga5000
Copy link

@csga5000 csga5000 commented Sep 26, 2018

@dnfield I see, that makes sense. So you're saying that the plan here is to support it via third party package to keep the engine small and doing so won't really worsen performance or anything.

@rashedmyt
Copy link

@rashedmyt rashedmyt commented Nov 14, 2018

@dnfield does flutter_svg support animated vector drawables?

@dnfield
Copy link
Member Author

@dnfield dnfield commented Nov 14, 2018

Not at this point

@rashedmyt
Copy link

@rashedmyt rashedmyt commented Nov 14, 2018

Is that in plans.. can we hope for that in the future?

@dnfield
Copy link
Member Author

@dnfield dnfield commented Nov 14, 2018

It should be possible but I don't have any kind of estimate around it yet. I'd be happy to take PRs for it, and there is some rudimentary AVD support

@rashedmyt
Copy link

@rashedmyt rashedmyt commented Nov 14, 2018

Oh really.. Can you point me to it.. I would like to see if that can help me generate the animation I require for my application.. If possible an example too 😛

@dnfield
Copy link
Member Author

@dnfield dnfield commented Nov 14, 2018

https://github.com/dnfield/flutter_svg/blob/master/lib/avd.dart - but as I said, it's very basic and not well tested. It can render a very simple AVD, which you can see in the example app in that repo.

@rashedmyt
Copy link

@rashedmyt rashedmyt commented Nov 14, 2018

thanks.. I'll look into it and give feedback

@rashedmyt
Copy link

@rashedmyt rashedmyt commented Nov 14, 2018

I tried with Animated Vector Drawable format but it throws some exception regarding single frame (something like that), Also tried the Vector Drawable format which worked nicely with a single unhandled exception of clip-path (Idk what that means but the app didn't crash or throw some weird red errors at screen though) .. Now I can just display my static vector logo without any hiccups (hoping avd.dart has support for both android(tested) and ios)...

Thanks a lot for the guidance and for this awesome library.. hoping for animations in future releases

Keep up the awesome work

@dnfield
Copy link
Member Author

@dnfield dnfield commented Nov 14, 2018

I would expect animated ones to fail right now - feel free to file an issue for that on the repo.

Clip paths are not implemented for AVDs right now, but should be similar to the SVG implementation. Could file an issue for that too if you like :)

@lzxb
Copy link

@lzxb lzxb commented May 5, 2019

I also need support for svg

@Hixie
Copy link
Member

@Hixie Hixie commented May 28, 2019

I'm curious about how the flutter_svg package has worked for people who are asking for this feature. Is it sufficient? What do you need that it doesn't support?

@cachapa
Copy link

@cachapa cachapa commented May 28, 2019

User of flutter_svg here: it's been working just fine for us. We haven't tried loading any complex SVGs yet, but the ones we tried worked flawlessly.
I still need to test shadows and gradients. If those work, then it's already a step up from Android vector drawables.

@dnfield
Copy link
Member Author

@dnfield dnfield commented May 28, 2019

Gradients should work with flutter_svg - there are a couple unusual permutations that are not implemented though.

Shadows are frequently created in SVGs using filter effects and blurs, which is not currently implemented in flutter_svg. That's been on my list for a while though.

@IchordeDionysos
Copy link

@IchordeDionysos IchordeDionysos commented May 29, 2019

Masking is currently not available, which makes displaying complex company logos, like our's not possible.
We are using flutter_svg anyways for simpler SVGs

@SAGARSURI
Copy link

@SAGARSURI SAGARSURI commented Jul 14, 2019

Any update on this?

@Hixie
Copy link
Member

@Hixie Hixie commented Jul 15, 2019

@SAGARSURI What kind of update are you looking for? Does flutter_svg work for you?

@dnfield
Copy link
Member Author

@dnfield dnfield commented Jul 18, 2019

I had been thinking of leaving this open until I supported one or two more features (mainly filter effects) that seem good for SVG.

That said, I'm really not really sure when I'll get to that, and although people have asked for it no one seems to be banging down the door for it. I'd like to implement them at some point, or I'd be happy to review a PR that implements them (I think at this point all of the engine bits we'd need are in place for them - I had been waiting on color filters and image filters on paint, which are both now available).

There are still a few odd bugs in there, but in general the library seems like it's serving its purpose. I'm inclined to just close this issue at this point, unless we think we want it open to de-dupe to.

@chunhtai
Copy link
Contributor

@chunhtai chunhtai commented Jul 25, 2019

If we have no intention of integrating this into flutter/flutter, we should close this. Any enhancement and bug should be tracked in the flutter_svg repo instead for easy tracking

@dnfield
Copy link
Member Author

@dnfield dnfield commented Jul 25, 2019

I agree. No one is banging down my door for the features that I still want to implement, and there are definitely some issues remaining in it but I don't see a need for this issue (short of deduplicating, which we can still do with it closed).

@dnfield dnfield closed this Jul 25, 2019
@rodydavis
Copy link
Contributor

@rodydavis rodydavis commented Oct 25, 2019

Works for me on Flutter Web! Source: https://github.com/mylisabox/remote_ui/blob/master/packages/remote_image_button/lib/src/widgets/image_svg_web.dart

import 'dart:ui' as ui;
import 'dart:html';

import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';

Widget createImageSvg({
  String url,
  double width,
  double height,
  BoxFit fit,
}) {
  return ImageSvgWeb(
    url: url,
    width: width,
    height: height,
    fit: fit,
  );
}

class ImageSvgWeb extends HookWidget {
  final String url;
  final double height;
  final double width;
  final BoxFit fit;

  const ImageSvgWeb({Key key, this.url, this.height, this.width, this.fit})
      : super(key: key);

  @override
  Widget build(BuildContext context) {
    final id = useMemoized(() => UniqueKey().toString());

    useEffect(() {
      ui.platformViewRegistry.registerViewFactory('img-svg-$id', (int viewId) {
        final element = ImageElement(
          src: url,
          height: height.toInt(),
          width: width.toInt(),
        );
        return element;
      });
      return () {};
    }, const []);

    return HtmlElementView(
      viewType: 'img-svg-$id',
    );
  }
}
@rfitz123
Copy link

@rfitz123 rfitz123 commented Jul 11, 2020

I have been unsuccessful in getting flutter_svg to properly display a dropshadow on the SVG. Is there a fix coming and/or a workaround for now?

@IchordeDionysos
Copy link

@IchordeDionysos IchordeDionysos commented Jul 12, 2020

@rfitz123 You should open an issue over at https://github.com/dnfield/flutter_svg

@softmarshmallow
Copy link

@softmarshmallow softmarshmallow commented Nov 12, 2020

What is the main reason this not being implemented natively?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet