Fix stroke attribute handling when gradients or groups are involved (#…

…671)
dnfield · Mar 1, 2022 · 943924c · 943924c
1 parent d0fd7a1
commit 943924c
Show file tree

Hide file tree

Showing 13 changed files with 198 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # CHANGES
 
+## 1.1.0
+
+- Respect stroke* properties when a paint definition is used for a stroke.
+- Respect stroke* properties from groups with no `@stroke` property.
+
 ## 1.0.3+1
 
 - Fix bugs in picture disposal.

diff --git a/example/android/app/build.gradle b/example/android/app/build.gradle
@@ -26,7 +26,7 @@ apply plugin: 'kotlin-android'
 apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle"
 
 android {
-    compileSdkVersion 30
+    compileSdkVersion 31
 
     sourceSets {
         main.java.srcDirs += 'src/main/kotlin'

diff --git a/example/android/build.gradle b/example/android/build.gradle
@@ -1,5 +1,7 @@
 buildscript {
-    ext.kotlin_version = '1.3.50'
+    // Do not copy this. It's strictly because this application does not care
+    // about Kotlin version and wants to always do its best to work.
+    ext.kotlin_version = '+'
     repositories {
         google()
         jcenter()

diff --git a/example/linux/flutter/generated_plugins.cmake b/example/linux/flutter/generated_plugins.cmake
@@ -5,6 +5,9 @@
 list(APPEND FLUTTER_PLUGIN_LIST
 )
 
+list(APPEND FLUTTER_FFI_PLUGIN_LIST
+)
+
 set(PLUGIN_BUNDLED_LIBRARIES)
 
 foreach(plugin ${FLUTTER_PLUGIN_LIST})
@@ -13,3 +16,8 @@ foreach(plugin ${FLUTTER_PLUGIN_LIST})
   list(APPEND PLUGIN_BUNDLED_LIBRARIES $<TARGET_FILE:${plugin}_plugin>)
   list(APPEND PLUGIN_BUNDLED_LIBRARIES ${${plugin}_bundled_libraries})
 endforeach(plugin)
+
+foreach(ffi_plugin ${FLUTTER_FFI_PLUGIN_LIST})
+  add_subdirectory(flutter/ephemeral/.plugin_symlinks/${ffi_plugin}/linux plugins/${ffi_plugin})
+  list(APPEND PLUGIN_BUNDLED_LIBRARIES ${${ffi_plugin}_bundled_libraries})
+endforeach(ffi_plugin)
diff --git a/example/windows/flutter/generated_plugins.cmake b/example/windows/flutter/generated_plugins.cmake
@@ -5,6 +5,9 @@
 list(APPEND FLUTTER_PLUGIN_LIST
 )
 
+list(APPEND FLUTTER_FFI_PLUGIN_LIST
+)
+
 set(PLUGIN_BUNDLED_LIBRARIES)
 
 foreach(plugin ${FLUTTER_PLUGIN_LIST})
@@ -13,3 +16,8 @@ foreach(plugin ${FLUTTER_PLUGIN_LIST})
   list(APPEND PLUGIN_BUNDLED_LIBRARIES $<TARGET_FILE:${plugin}_plugin>)
   list(APPEND PLUGIN_BUNDLED_LIBRARIES ${${plugin}_bundled_libraries})
 endforeach(plugin)
+
+foreach(ffi_plugin ${FLUTTER_FFI_PLUGIN_LIST})
+  add_subdirectory(flutter/ephemeral/.plugin_symlinks/${ffi_plugin}/windows plugins/${ffi_plugin})
+  list(APPEND PLUGIN_BUNDLED_LIBRARIES ${${ffi_plugin}_bundled_libraries})
+endforeach(ffi_plugin)
diff --git a/golden/simple/use_opacity_grid.png b/golden/simple/use_opacity_grid.png
diff --git a/golden/wikimedia/chess_knight.png b/golden/wikimedia/chess_knight.png
diff --git a/lib/src/svg/parser_state.dart b/lib/src/svg/parser_state.dart
@@ -792,7 +792,7 @@ class SvgParserState {
     this.theme,
     this._key,
     this._warningsAsErrors,
-  )   
+  )
   // ignore: unnecessary_null_comparison
   : assert(events != null),
         _eventIterator = events.iterator;
@@ -1294,7 +1294,7 @@ class SvgParserState {
     DrawablePaint? parentStroke,
     Color? currentColor,
   ) {
-    final String rawStroke = getAttribute(attributes, 'stroke')!;
+    final String? rawStroke = getAttribute(attributes, 'stroke', def: null);
     final String? rawStrokeOpacity = getAttribute(
       attributes,
       'stroke-opacity',
@@ -1306,57 +1306,74 @@ class SvgParserState {
       opacity *= parseDouble(rawOpacity)!.clamp(0.0, 1.0);
     }
 
-    if (rawStroke.startsWith('url')) {
-      return _getDefinitionPaint(
+    final String? rawStrokeCap =
+        getAttribute(attributes, 'stroke-linecap', def: null);
+    final String? rawLineJoin =
+        getAttribute(attributes, 'stroke-linejoin', def: null);
+    final String? rawMiterLimit =
+        getAttribute(attributes, 'stroke-miterlimit', def: null);
+    final String? rawStrokeWidth =
+        getAttribute(attributes, 'stroke-width', def: null);
+
+    final String? anyStrokeAttribute = rawStroke ??
+        rawStrokeCap ??
+        rawLineJoin ??
+        rawMiterLimit ??
+        rawStrokeWidth;
+    if (anyStrokeAttribute == null && DrawablePaint.isEmpty(parentStroke)) {
+      return null;
+    } else if (rawStroke == 'none') {
+      return DrawablePaint.empty;
+    }
+
+    DrawablePaint? definitionPaint;
+    Color? strokeColor;
+    if (rawStroke?.startsWith('url') == true) {
+      definitionPaint = _getDefinitionPaint(
         _key,
         PaintingStyle.stroke,
-        rawStroke,
+        rawStroke!,
         _definitions,
         bounds,
         opacity: opacity,
       );
+      strokeColor = definitionPaint.color;
+    } else {
+      strokeColor = parseColor(rawStroke);
     }
-    if (rawStroke == '' && DrawablePaint.isEmpty(parentStroke)) {
-      return null;
-    }
-    if (rawStroke == 'none') {
-      return DrawablePaint.empty;
-    }
-
-    final String? rawStrokeCap = getAttribute(attributes, 'stroke-linecap');
-    final String? rawLineJoin = getAttribute(attributes, 'stroke-linejoin');
-    final String? rawMiterLimit = getAttribute(attributes, 'stroke-miterlimit');
-    final String? rawStrokeWidth = getAttribute(attributes, 'stroke-width');
 
     final DrawablePaint paint = DrawablePaint(
       PaintingStyle.stroke,
-      color: rawStroke == ''
-          ? (parentStroke?.color ?? colorBlack).withOpacity(opacity)
-          : (parseColor(rawStroke) ??
-                  currentColor ??
-                  parentStroke?.color ??
-                  colorBlack)
-              .withOpacity(opacity),
-      strokeCap: rawStrokeCap == 'null'
-          ? parentStroke?.strokeCap ?? StrokeCap.butt
-          : StrokeCap.values.firstWhere(
-              (StrokeCap sc) => sc.toString() == 'StrokeCap.$rawStrokeCap',
-              orElse: () => StrokeCap.butt,
-            ),
-      strokeJoin: rawLineJoin == ''
-          ? parentStroke?.strokeJoin ?? StrokeJoin.miter
-          : StrokeJoin.values.firstWhere(
-              (StrokeJoin sj) => sj.toString() == 'StrokeJoin.$rawLineJoin',
-              orElse: () => StrokeJoin.miter,
-            ),
-      strokeMiterLimit: rawMiterLimit == ''
-          ? parentStroke?.strokeMiterLimit ?? 4.0
-          : parseDouble(rawMiterLimit),
-      strokeWidth: rawStrokeWidth == ''
-          ? parentStroke?.strokeWidth ?? 1.0
-          : parseDoubleWithUnits(rawStrokeWidth),
+      color: (strokeColor ??
+              currentColor ??
+              parentStroke?.color ??
+              definitionPaint?.color)
+          ?.withOpacity(opacity),
+      strokeCap: StrokeCap.values.firstWhere(
+        (StrokeCap sc) => sc.toString() == 'StrokeCap.$rawStrokeCap',
+        orElse: () =>
+            parentStroke?.strokeCap ??
+            definitionPaint?.strokeCap ??
+            StrokeCap.butt,
+      ),
+      strokeJoin: StrokeJoin.values.firstWhere(
+        (StrokeJoin sj) => sj.toString() == 'StrokeJoin.$rawLineJoin',
+        orElse: () =>
+            parentStroke?.strokeJoin ??
+            definitionPaint?.strokeJoin ??
+            StrokeJoin.miter,
+      ),
+      strokeMiterLimit: parseDouble(rawMiterLimit) ??
+          parentStroke?.strokeMiterLimit ??
+          definitionPaint?.strokeMiterLimit ??
+          4.0,
+      strokeWidth: parseDoubleWithUnits(rawStrokeWidth) ??
+          parentStroke?.strokeWidth ??
+          definitionPaint?.strokeWidth ??
+          1.0,
     );
-    return paint;
+
+    return DrawablePaint.merge(definitionPaint, paint);
   }
 
   /// Parses a `fill` attribute.

diff --git a/lib/src/vector_drawable.dart b/lib/src/vector_drawable.dart
@@ -207,19 +207,20 @@ class DrawablePaint {
     assert(a.style == b!.style,
         'Cannot merge Paints with different PaintStyles; got:\na: $a\nb: $b.');
 
+    b = b!;
     return DrawablePaint(
-      a.style ?? b!.style,
-      color: a.color ?? b!.color,
-      shader: a.shader ?? b!.shader,
-      blendMode: a.blendMode ?? b!.blendMode,
-      colorFilter: a.colorFilter ?? b!.colorFilter,
-      isAntiAlias: a.isAntiAlias ?? b!.isAntiAlias,
-      filterQuality: a.filterQuality ?? b!.filterQuality,
-      maskFilter: a.maskFilter ?? b!.maskFilter,
-      strokeCap: a.strokeCap ?? b!.strokeCap,
-      strokeJoin: a.strokeJoin ?? b!.strokeJoin,
-      strokeMiterLimit: a.strokeMiterLimit ?? b!.strokeMiterLimit,
-      strokeWidth: a.strokeWidth ?? b!.strokeWidth,
+      a.style ?? b.style,
+      color: a.color ?? b.color,
+      shader: a.shader ?? b.shader,
+      blendMode: a.blendMode ?? b.blendMode,
+      colorFilter: a.colorFilter ?? b.colorFilter,
+      isAntiAlias: a.isAntiAlias ?? b.isAntiAlias,
+      filterQuality: a.filterQuality ?? b.filterQuality,
+      maskFilter: a.maskFilter ?? b.maskFilter,
+      strokeCap: a.strokeCap ?? b.strokeCap,
+      strokeJoin: a.strokeJoin ?? b.strokeJoin,
+      strokeMiterLimit: a.strokeMiterLimit ?? b.strokeMiterLimit,
+      strokeWidth: a.strokeWidth ?? b.strokeWidth,
     );
   }
 
@@ -230,7 +231,7 @@ class DrawablePaint {
 
   /// Returns whether this paint is null or equivalent to SVG's "none".
   static bool isEmpty(DrawablePaint? paint) {
-    return paint == null || paint == empty;
+    return paint == null || identical(empty, paint) || paint.color == null;
   }
 
   /// The color to use for this paint when stroking or filling a shape.
@@ -1290,12 +1291,12 @@ class DrawableShape implements DrawableStyleable {
       if (style.mask != null) {
         canvas.saveLayer(null, Paint());
       }
-      if (style.fill?.style != null) {
+      if (style.fill?.color != null) {
         assert(style.fill!.style == PaintingStyle.fill);
         canvas.drawPath(path, style.fill!.toFlutterPaint());
       }
 
-      if (style.stroke?.style != null) {
+      if (style.stroke?.color != null) {
         assert(style.stroke!.style == PaintingStyle.stroke);
         if (style.dashArray != null &&
             !identical(style.dashArray, DrawableStyle.emptyDashArray)) {

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -1,7 +1,7 @@
 name: flutter_svg
 description: An SVG rendering and widget library for Flutter, which allows painting and displaying Scalable Vector Graphics 1.1 files.
 homepage: https://github.com/dnfield/flutter_svg
-version: 1.0.3+1
+version: 1.1.0
 
 dependencies:
   flutter:

diff --git a/test/golden_widget/text_color_filter.png b/test/golden_widget/text_color_filter.png
diff --git a/test/svg_parsers_test.dart b/test/svg_parsers_test.dart
@@ -1112,4 +1112,75 @@ BAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" x="1ex" y="0.5ex" width="2ex" height="1.5ex" /
     expect(root.compatibilityTester.isCompatible(oldTheme, newTheme), true);
     expect(root.compatibilityTester.isCompatible(oldTheme, newTheme2), false);
   });
+
+  test('Preserves stroke-width when gradient is used', () async {
+    const String svgStr = '''
+<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
+  <linearGradient id="gradient" gradientUnits="userSpaceOnUse" x1="0" x2="100" y1="50" y2="50">
+    <stop offset=".0" stop-color="#0000ff" />
+    <stop offset=".3" stop-color="#00ccff" />
+    <stop offset=".6" stop-color="#00ffd5" />
+    <stop offset=".9" stop-color="#00ff00" />
+  </linearGradient>
+  <circle cx="50" cy="50" r="40" stroke="url(#gradient)" stroke-linecap="round" stroke-width="10" />
+</svg>''';
+
+    final SvgParser parser = SvgParser();
+    final DrawableRoot root = await parser.parse(svgStr);
+
+    expect(root.children.length, 1);
+    final DrawableShape circle = root.children.first as DrawableShape;
+    expect(circle.style.stroke!.strokeWidth, 10);
+    expect(circle.style.stroke!.strokeMiterLimit, 4);
+  });
+
+  test('Preserves stroke properties from group with no "stroke"', () async {
+    const String svgStr = '''
+<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
+    <linearGradient id="gradient" gradientUnits="userSpaceOnUse" x1="0" x2="100" y1="50" y2="50">
+        <stop offset=".0" stop-color="#0000ff" />
+        <stop offset=".3" stop-color="#00ccff" />
+        <stop offset=".6" stop-color="#00ffd5" />
+        <stop offset=".9" stop-color="#00ff00" />
+    </linearGradient>
+    <g fill="none" stroke-miterlimit="8" stroke-width="10">
+        <circle cx="50" cy="50" r="40" stroke="url(#gradient)" stroke-linecap="round" />
+    </g>
+</svg>''';
+
+    final SvgParser parser = SvgParser();
+    final DrawableRoot root = await parser.parse(svgStr);
+
+    expect(root.children.length, 1);
+    expect(root.children.length, 1);
+    final DrawableGroup group = root.children.first as DrawableGroup;
+    final DrawableShape circle = group.children!.first as DrawableShape;
+    expect(circle.style.stroke!.strokeWidth, 10);
+    expect(circle.style.stroke!.strokeMiterLimit, 8);
+  });
+
+  test('Takes stroke properties from shape when group has them', () async {
+    const String svgStr = '''
+<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
+    <linearGradient id="gradient" gradientUnits="userSpaceOnUse" x1="0" x2="100" y1="50" y2="50">
+        <stop offset=".0" stop-color="#0000ff" />
+        <stop offset=".3" stop-color="#00ccff" />
+        <stop offset=".6" stop-color="#00ffd5" />
+        <stop offset=".9" stop-color="#00ff00" />
+    </linearGradient>
+    <g fill="none" stroke-miterlimit="8" stroke-width="10">
+        <circle cx="50" cy="50" r="40" stroke="url(#gradient)" stroke-linecap="round" stroke-width="5" />
+    </g>
+</svg>''';
+
+    final SvgParser parser = SvgParser();
+    final DrawableRoot root = await parser.parse(svgStr);
+
+    expect(root.children.length, 1);
+    expect(root.children.length, 1);
+    final DrawableGroup group = root.children.first as DrawableGroup;
+    final DrawableShape circle = group.children!.first as DrawableShape;
+    expect(circle.style.stroke!.strokeWidth, 5);
+    expect(circle.style.stroke!.strokeMiterLimit, 8);
+  });
 }
diff --git a/test/vector_drawable_test.dart b/test/vector_drawable_test.dart
@@ -62,4 +62,30 @@ void main() {
 
     expect(info.createLayer().isComplexHint, true);
   });
+
+  test('mergeAndBlend gets strokeWidth right', () async {
+    final DrawableRoot root = await svg.fromSvgString(
+      '''
+<svg viewBox="0 0 44 78" fill="none" xmlns="http://www.w3.org/2000/svg">
+  <path d="M5 10L20 20L 10 20Z" stroke="white" stroke-width="2" />
+</svg>
+''',
+      'test',
+    );
+
+    final DrawablePaint strokePaintA =
+        (root.children.first as DrawableShape).style.stroke!;
+    final DrawableRoot mergedRoot = root.mergeStyle(
+      const DrawableStyle(
+        stroke: DrawablePaint(
+          PaintingStyle.stroke,
+          color: Color(0xFFABCDEF),
+        ),
+      ),
+    );
+
+    final DrawablePaint strokePaintB =
+        (mergedRoot.children.first as DrawableShape).style.stroke!;
+    expect(strokePaintA.strokeWidth, strokePaintB.strokeWidth);
+  });
 }