diff --git a/packages/vector_graphics_compiler/CHANGELOG.md b/packages/vector_graphics_compiler/CHANGELOG.md index b43c464030a..738b0afd563 100644 --- a/packages/vector_graphics_compiler/CHANGELOG.md +++ b/packages/vector_graphics_compiler/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 1.1.20 +* Fixes color parsing for modern rgb and rgba CSS syntax. * Updates minimum supported SDK version to Flutter 3.35/Dart 3.9. ## 1.1.19 diff --git a/packages/vector_graphics_compiler/lib/src/svg/colors.dart b/packages/vector_graphics_compiler/lib/src/svg/colors.dart index 718cd42b184..5d9b3e6ad66 100644 --- a/packages/vector_graphics_compiler/lib/src/svg/colors.dart +++ b/packages/vector_graphics_compiler/lib/src/svg/colors.dart @@ -157,3 +157,145 @@ const Map namedColors = { 'yellow': Color.fromARGB(255, 255, 255, 0), 'yellowgreen': Color.fromARGB(255, 154, 205, 50), }; + +/// Parses a CSS `rgb()` or `rgba()` function color string and returns a Color. +/// +/// The [colorString] should be the full color string including the function +/// name (`rgb` or `rgba`) and parentheses. +/// +/// Both `rgb()` and `rgba()` accept the same syntax variations: +/// - `rgb(R G B)` or `rgba(R G B)` - modern space-separated +/// - `rgb(R G B / A)` or `rgba(R G B / A)` - modern with slash before alpha +/// - `rgb(R,G,B)` or `rgba(R,G,B)` - legacy comma-separated +/// - `rgb(R,G,B,A)` or `rgba(R,G,B,A)` - legacy with alpha +/// +/// Throws [ArgumentError] if the color string is invalid. +Color parseRgbFunction(String colorString) { + final String content = colorString.substring( + colorString.indexOf('(') + 1, + colorString.indexOf(')'), + ); + + if (content.isEmpty) { + throw ArgumentError.value(colorString, 'colorString', 'Empty content'); + } + final List stringRgbValues; + final String? stringAlphaValue; + + final List commaSplit = content + .split(',') + .map((String value) => value.trim()) + .toList(); + + if (commaSplit.length > 1) { + // We are dealing with comma-separated syntax + (stringAlphaValue, stringRgbValues) = switch (commaSplit.length) { + 3 => (null, commaSplit), + 4 => (commaSplit.removeLast(), commaSplit), + _ => throw ArgumentError.value( + colorString, + 'colorString', + 'Expected 3 or 4 values, got ${commaSplit.length}', + ), + }; + } else { + final List slashSplit = content + .split('/') + .map((String value) => value.trim()) + .toList(); + + final ( + String tempStringRgbValues, + String? tempStringAlphaValue, + ) = switch (slashSplit.length) { + 1 => (slashSplit.first, null), + 2 => (slashSplit.first, slashSplit.last), + _ => throw ArgumentError.value( + colorString, + 'colorString', + 'Multiple slashes not allowed', + ), + }; + + final List rgbSpaceSplit = tempStringRgbValues + .split(' ') + .map((String value) => value.trim()) + .where((String value) => value.isNotEmpty) + .toList(); + + if (rgbSpaceSplit.length != 3) { + throw ArgumentError.value( + colorString, + 'colorString', + 'Expected 3 space-separated RGB values', + ); + } + + stringRgbValues = rgbSpaceSplit; + stringAlphaValue = tempStringAlphaValue; + } + + final List rgbIntValues = stringRgbValues + .map( + (String value) => _parseRgbaFunctionComponent( + isAlpha: false, + rawComponentValue: value, + originalColorString: colorString, + ), + ) + .toList(); + + final int a = stringAlphaValue == null + ? 255 + : _parseRgbaFunctionComponent( + isAlpha: true, + rawComponentValue: stringAlphaValue, + originalColorString: colorString, + ); + + return Color.fromARGB(a, rgbIntValues[0], rgbIntValues[1], rgbIntValues[2]); +} + +/// Parses a single RGB/RGBA component value and returns an integer 0-255. +/// +/// The [rawComponentValue] can be: +/// - A percentage (e.g., "50%") - converted to 0-255 range +/// - A decimal number (e.g., "128" or "128.5") - clamped to 0-255 for RGB +/// - For alpha (index 3): decimal 0-1 range, converted to 0-255 +/// +/// Out-of-bounds values are clamped rather than rejected. +/// +/// Throws [ArgumentError] if the value cannot be parsed as a number. +int _parseRgbaFunctionComponent({ + required bool isAlpha, + required String rawComponentValue, + required String originalColorString, +}) { + if (rawComponentValue.endsWith('%')) { + final String numPart = rawComponentValue.substring( + 0, + rawComponentValue.length - 1, + ); + final double? percent = double.tryParse(numPart); + if (percent == null) { + throw ArgumentError.value( + originalColorString, + 'originalColorString', + 'invalid percentage "$rawComponentValue"', + ); + } + return (percent.clamp(0, 100) * 2.55).round(); + } + final double? value = double.tryParse(rawComponentValue); + if (value == null) { + throw ArgumentError.value( + originalColorString, + 'originalColorString', + 'invalid value "$rawComponentValue"', + ); + } + if (isAlpha) { + return (value.clamp(0, 1) * 255).round(); + } + return value.clamp(0, 255).round(); +} diff --git a/packages/vector_graphics_compiler/lib/src/svg/parser.dart b/packages/vector_graphics_compiler/lib/src/svg/parser.dart index b8f67adc89c..a9746f76a54 100644 --- a/packages/vector_graphics_compiler/lib/src/svg/parser.dart +++ b/packages/vector_graphics_compiler/lib/src/svg/parser.dart @@ -1372,21 +1372,10 @@ class SvgParser { } } - // handle rgba() colors e.g. rgba(255, 255, 255, 1.0) - if (colorString.toLowerCase().startsWith('rgba')) { - final List rawColorElements = colorString - .substring(colorString.indexOf('(') + 1, colorString.indexOf(')')) - .split(',') - .map((String rawColor) => rawColor.trim()) - .toList(); - - final double opacity = parseDouble(rawColorElements.removeLast())!; - - final List rgb = rawColorElements - .map((String rawColor) => int.parse(rawColor)) - .toList(); - - return Color.fromRGBO(rgb[0], rgb[1], rgb[2], opacity); + // handle rgba() colors e.g. rgb(255, 255, 255) and rgba(255, 255, 255, 1.0) + // https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/rgb + if (colorString.toLowerCase().startsWith('rgb')) { + return parseRgbFunction(colorString); } // Conversion code from: https://github.com/MichaelFenwick/Color, thanks :) @@ -1456,26 +1445,6 @@ class SvgParser { ); } - // handle rgb() colors e.g. rgb(255, 255, 255) - if (colorString.toLowerCase().startsWith('rgb')) { - final List rgb = colorString - .substring(colorString.indexOf('(') + 1, colorString.indexOf(')')) - .split(',') - .map((String rawColor) { - rawColor = rawColor.trim(); - if (rawColor.endsWith('%')) { - rawColor = rawColor.substring(0, rawColor.length - 1); - return (parseDouble(rawColor)! * 2.55).round(); - } - return int.parse(rawColor); - }) - .toList(); - - // rgba() isn't really in the spec, but Firefox supported it at one point so why not. - final int a = rgb.length > 3 ? rgb[3] : 255; - return Color.fromARGB(a, rgb[0], rgb[1], rgb[2]); - } - // handle named colors ('red', 'green', etc.). final Color? namedColor = namedColors[colorString]; if (namedColor != null) { diff --git a/packages/vector_graphics_compiler/pubspec.yaml b/packages/vector_graphics_compiler/pubspec.yaml index 3e6911fc9a3..43d627e60eb 100644 --- a/packages/vector_graphics_compiler/pubspec.yaml +++ b/packages/vector_graphics_compiler/pubspec.yaml @@ -2,7 +2,7 @@ name: vector_graphics_compiler description: A compiler to convert SVGs to the binary format used by `package:vector_graphics`. repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics_compiler issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22 -version: 1.1.19 +version: 1.1.20 executables: vector_graphics_compiler: diff --git a/packages/vector_graphics_compiler/test/parsers_test.dart b/packages/vector_graphics_compiler/test/parsers_test.dart index 6062682368a..1750ae4dd72 100644 --- a/packages/vector_graphics_compiler/test/parsers_test.dart +++ b/packages/vector_graphics_compiler/test/parsers_test.dart @@ -21,11 +21,202 @@ void main() { parser.parseColor('#ABCDEF', attributeName: 'foo', id: null), const Color.fromARGB(255, 0xAB, 0xCD, 0xEF), ); - // RGBA in svg/css, ARGB in this library. - expect( - parser.parseColor('#ABCDEF88', attributeName: 'foo', id: null), - const Color.fromARGB(0x88, 0xAB, 0xCD, 0xEF), - ); + }); + + group('Colors - svg/css', () { + final parser = SvgParser('', const SvgTheme(), 'test_key', true, null); + + group('with no opacity', () { + const rgbContentVariations = [ + // Legacy syntax (comma-separated) + '171, 205, 239', + '171,205,239', + // Modern syntax (space-separated) + '171 205 239', + // Percentage values + '67% 80.5% 93.7%', + // Mixed percentage and decimal (space-separated) + '67% 205 93.7%', + // Decimal RGB values + '171.1 205.1 238.9', + ]; + + final List rgbaVariations = [ + '#ABCDEF', + ...rgbContentVariations.map((rgb) => 'rgba($rgb)'), + ...rgbContentVariations.map((rgb) => 'rgb($rgb)'), + ]; + + for (final rgba in rgbaVariations) { + test(rgba, () { + expect( + parser.parseColor(rgba, attributeName: 'foo', id: null), + const Color.fromARGB(0xFF, 0xAB, 0xCD, 0xEF), + ); + }); + } + }); + + group('with opacity', () { + const rgbContentVariations = [ + // Legacy syntax (comma-separated) + '171, 205, 239, 0.53', + '171,205,239,0.53', + // Modern syntax (space-separated with slash before alpha) + '171 205 239 / 53%', + '171 205 239 / 0.53', + '171 205 239 / .53', // leading dot + '171 205 239 / 0.53', + // Percentage RGB with slash alpha + '67% 80.5% 93.7% / 53%', + // Mixed percentage and decimal RGB (space-separated) with slash alpha + '67% 205 93.7% / 53%', + // Decimal RGB values with percentage alpha + '171.1 205.1 238.9 / 53%', + // Decimal RGB values with decimal alpha + '171.1 205.1 238.9 / 0.53', + ]; + + final List rgbaVariations = [ + '#ABCDEF87', + ...rgbContentVariations.map((rgb) => 'rgba($rgb)'), + ...rgbContentVariations.map((rgb) => 'rgb($rgb)'), + ]; + + for (final rgba in rgbaVariations) { + test(rgba, () { + expect( + parser.parseColor(rgba, attributeName: 'foo', id: null), + const Color.fromARGB(0x87, 0xAB, 0xCD, 0xEF), + ); + }); + } + }); + + group('with values out of bounds', () { + // RGB values > 255 clamp to 255, negative values clamp to 0 + // Percentages > 100% clamp to 100%, negative percentages clamp to 0% + // Alpha values > 1 clamp to 1, negative alpha clamps to 0 + + test('rgb(256.9, 0, 256) clamps RGB to 255', () { + expect( + parser.parseColor( + 'rgb(256.9, 0, 256)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(255, 255, 0, 255), + ); + }); + + test('rgb(-50, 300, -100) clamps negative to 0 and >255 to 255', () { + expect( + parser.parseColor( + 'rgb(-50, 300, -100)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(255, 0, 255, 0), + ); + }); + + test('rgb(120%, -10%, 200%) clamps percentages', () { + expect( + parser.parseColor( + 'rgb(120%, -10%, 200%)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(255, 255, 0, 255), + ); + }); + + test('rgb(255, 0, 255, -0.5) negative alpha clamps to 0', () { + expect( + parser.parseColor( + 'rgb(255, 0, 255, -0.5)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(0, 255, 0, 255), + ); + }); + + test('rgb(128, 128, 128, 2.5) alpha > 1 clamps to 1', () { + expect( + parser.parseColor( + 'rgb(128, 128, 128, 2.5)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(255, 128, 128, 128), + ); + }); + + test('rgb(999 -50 300 / 150%) clamps all values', () { + expect( + parser.parseColor( + 'rgb(999 -50 300 / 150%)', + attributeName: 'foo', + id: null, + ), + const Color.fromARGB(255, 255, 0, 255), + ); + }); + }); + + group('invalid syntax', () { + const rgbContentVariations = [ + // 4 space-separated values without slash (modern syntax requires slash before alpha) + '171.1 205.1 238.9 53%', + '255 255 255 0.5', + '255 122 127 80%', + // Mixed comma and slash separators (not allowed) + '171.1,205.1 238.9/53%', + '255, 255, 255 / 0.5', + // Space-separated values after comma (not allowed) + '171.1,205.1 238.9, 53%', + '129% ,09% 255%,5.5', + // Single comma with 4 values (legacy alpha needs 2+ commas) + '129% 09% 255%,5.5', + // Empty values (double comma, leading comma, trailing comma) + '67%,,93.7%, 53%', + '10,,10', + '50,90,,0', + '255, 128, 0,', + // Slash between RGB values (slash only allowed before alpha) + '255 / 255 / 255', + // Too few values + '255 255', + // Too many values + '255 255 255 128 64', + // Missing alpha after slash + '255 255 255 /', + // Mixed separators: spaces before first comma, then commas (no alpha) + // Note: This actually parses correctly on css(web) but it's not valid as per the spec. + '171 205,239', + // Mixed separators: spaces before first comma, then commas (decimal alpha) + // Note: This actually parses correctly on css(web) but it's not valid as per the spec. + '171 205,239, 0.53', + // Mixed separators: spaces before first comma, then commas (percentage alpha) + // Note: This actually parses correctly on css(web) but it's not valid as per the spec. + '171 205,239, 53%', + ]; + + final List rgbaVariations = [ + ...rgbContentVariations.map((rgb) => 'rgba($rgb)'), + ...rgbContentVariations.map((rgb) => 'rgb($rgb)'), + ]; + + for (final rgba in rgbaVariations) { + test(rgba, () { + expect( + () => parser.parseColor(rgba, attributeName: 'foo', id: null), + throwsArgumentError, + ); + }); + } + }); }); test('Colors - mapped', () async {