From a51ed1392f30eee131185b4d1a38cac08a44713f Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 20 Apr 2023 10:51:06 -0400 Subject: [PATCH 1/2] [web] Improve null safety for color->css --- lib/web_ui/lib/src/engine/canvas_pool.dart | 6 +++--- .../lib/src/engine/html/bitmap_canvas.dart | 12 ++++++------ lib/web_ui/lib/src/engine/html/clip.dart | 2 +- .../lib/src/engine/html/color_filter.dart | 12 ++++++------ lib/web_ui/lib/src/engine/html/dom_canvas.dart | 10 +++++----- lib/web_ui/lib/src/engine/html/painting.dart | 2 +- .../lib/src/engine/html/shaders/shader.dart | 8 ++++---- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 +++++----- lib/web_ui/lib/src/engine/util.dart | 17 ++++++----------- .../test/html/path_to_svg_golden_test.dart | 4 ++-- 10 files changed, 39 insertions(+), 44 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart index c264080bf82a8..ea6cc11303af7 100644 --- a/lib/web_ui/lib/src/engine/canvas_pool.dart +++ b/lib/web_ui/lib/src/engine/canvas_pool.dart @@ -541,7 +541,7 @@ class CanvasPool extends _SaveStackTracking { void drawColor(ui.Color color, ui.BlendMode blendMode) { final DomCanvasRenderingContext2D ctx = context; contextHandle.blendMode = blendMode; - contextHandle.fillStyle = colorToCssString(color); + contextHandle.fillStyle = color.toCssString(); contextHandle.strokeStyle = ''; ctx.beginPath(); // Fill a virtually infinite rect with the color. @@ -997,7 +997,7 @@ class ContextStateHandle { } } } else { - final String? colorString = colorValueToCssString(paint.color); + final String colorString = colorValueToCssString(paint.color); fillStyle = colorString; strokeStyle = colorString; } @@ -1019,7 +1019,7 @@ class ContextStateHandle { context.save(); context.shadowBlur = convertSigmaToRadius(maskFilter.webOnlySigma); // Shadow color must be fully opaque. - context.shadowColor = colorToCssString(ui.Color(paint.color).withAlpha(255)); + context.shadowColor = ui.Color(paint.color).withAlpha(255).toCssString(); // On the web a shadow must always be painted together with the shape // that casts it. In order to paint just the shadow, we offset the shape diff --git a/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart index 00948fcf4c068..4ebde4e3ab78e 100644 --- a/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart @@ -572,7 +572,7 @@ class BitmapCanvas extends EngineCanvas { void _applyFilter(DomElement element, SurfacePaintData paint) { if (paint.maskFilter != null) { final bool isStroke = paint.style == ui.PaintingStyle.stroke; - final String cssColor = colorValueToCssString(paint.color)!; + final String cssColor = colorValueToCssString(paint.color); final double sigma = paint.maskFilter!.webOnlySigma; if (browserEngine == BrowserEngine.webkit && !isStroke) { // A bug in webkit leaves artifacts when this element is animated @@ -808,7 +808,7 @@ class BitmapCanvas extends EngineCanvas { case ui.BlendMode.srcOver: style ..position = 'absolute' - ..backgroundColor = colorToCssString(filterColor)!; + ..backgroundColor = filterColor!.toCssString(); case ui.BlendMode.dst: case ui.BlendMode.dstIn: style @@ -820,7 +820,7 @@ class BitmapCanvas extends EngineCanvas { ..backgroundImage = "url('${image.imgElement.src}')" ..backgroundBlendMode = blendModeToCssMixBlendMode(colorFilterBlendMode) ?? '' - ..backgroundColor = colorToCssString(filterColor)!; + ..backgroundColor = filterColor!.toCssString(); break; } return imgElement; @@ -839,7 +839,7 @@ class BitmapCanvas extends EngineCanvas { final DomHTMLElement imgElement = _reuseOrCreateImage(image); imgElement.style.filter = 'url(#${svgFilter.id})'; if (colorFilterBlendMode == ui.BlendMode.saturation) { - imgElement.style.backgroundColor = colorToCssString(filterColor)!; + imgElement.style.backgroundColor = filterColor!.toCssString(); } return imgElement; } @@ -900,7 +900,7 @@ class BitmapCanvas extends EngineCanvas { if (shadows != null) { ctx.save(); for (final ui.Shadow shadow in shadows) { - ctx.shadowColor = colorToCssString(shadow.color); + ctx.shadowColor = shadow.color.toCssString(); ctx.shadowBlur = shadow.blurRadius; ctx.shadowOffsetX = shadow.offset.dx; ctx.shadowOffsetY = shadow.offset.dy; @@ -1009,7 +1009,7 @@ class BitmapCanvas extends EngineCanvas { final ui.Color color = ui.Color(paint.color); _canvasPool.contextHandle ..fillStyle = null - ..strokeStyle = colorToCssString(color); + ..strokeStyle = color.toCssString(); glRenderer!.drawHairline(ctx, positions); restore(); return; diff --git a/lib/web_ui/lib/src/engine/html/clip.dart b/lib/web_ui/lib/src/engine/html/clip.dart index dc826ff88a5b0..cc2303b7a1d76 100644 --- a/lib/web_ui/lib/src/engine/html/clip.dart +++ b/lib/web_ui/lib/src/engine/html/clip.dart @@ -231,7 +231,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface } void _applyColor() { - rootElement!.style.backgroundColor = colorToCssString(color)!; + rootElement!.style.backgroundColor = color.toCssString(); } @override diff --git a/lib/web_ui/lib/src/engine/html/color_filter.dart b/lib/web_ui/lib/src/engine/html/color_filter.dart index b0fc45605d9c8..353f82ce8e8cf 100644 --- a/lib/web_ui/lib/src/engine/html/color_filter.dart +++ b/lib/web_ui/lib/src/engine/html/color_filter.dart @@ -352,7 +352,7 @@ SvgFilter _srcInColorFilterToSvg(ui.Color? color) { result: 'destalpha', ); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); @@ -374,7 +374,7 @@ SvgFilter _srcInColorFilterToSvg(ui.Color? color) { SvgFilter _dstATopColorFilterToSvg(ui.Color? color) { final SvgFilterBuilder builder = SvgFilterBuilder(); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); @@ -390,7 +390,7 @@ SvgFilter _dstATopColorFilterToSvg(ui.Color? color) { SvgFilter _srcOutColorFilterToSvg(ui.Color? color) { final SvgFilterBuilder builder = SvgFilterBuilder(); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); @@ -406,7 +406,7 @@ SvgFilter _srcOutColorFilterToSvg(ui.Color? color) { SvgFilter _xorColorFilterToSvg(ui.Color? color) { final SvgFilterBuilder builder = SvgFilterBuilder(); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); @@ -425,7 +425,7 @@ SvgFilter _compositeColorFilterToSvg( ui.Color? color, double k1, double k2, double k3, double k4) { final SvgFilterBuilder builder = SvgFilterBuilder(); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); @@ -478,7 +478,7 @@ SvgFilter _blendColorFilterToSvg(ui.Color? color, SvgBlendMode svgBlendMode, {bool swapLayers = false}) { final SvgFilterBuilder builder = SvgFilterBuilder(); builder.setFeFlood( - floodColor: colorToCssString(color) ?? '', + floodColor: color?.toCssString() ?? '', floodOpacity: '1', result: 'flood', ); diff --git a/lib/web_ui/lib/src/engine/html/dom_canvas.dart b/lib/web_ui/lib/src/engine/html/dom_canvas.dart index 0e79b3e08baf8..b56f952f2b5f2 100644 --- a/lib/web_ui/lib/src/engine/html/dom_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/dom_canvas.dart @@ -60,7 +60,7 @@ class DomCanvas extends EngineCanvas with SaveElementStackTracking { ..right = '0' ..bottom = '0' ..left = '0' - ..backgroundColor = colorToCssString(color)!; + ..backgroundColor = color.toCssString(); currentElement.append(box); } @@ -257,7 +257,7 @@ DomHTMLElement buildDrawRectElement( ..transformOrigin = '0 0 0' ..transform = effectiveTransform; - String cssColor = colorValueToCssString(paint.color)!; + String cssColor = colorValueToCssString(paint.color); if (paint.maskFilter != null) { final double sigma = paint.maskFilter!.webOnlySigma; @@ -265,7 +265,7 @@ DomHTMLElement buildDrawRectElement( // A bug in webkit leaves artifacts when this element is animated // with filter: blur, we use boxShadow instead. style.boxShadow = '0px 0px ${sigma * 2.0}px $cssColor'; - cssColor = colorToCssString(blurColor(ui.Color(paint.color), sigma))!; + cssColor = blurColor(ui.Color(paint.color), sigma).toCssString(); } else { style.filter = 'blur(${sigma}px)'; } @@ -345,14 +345,14 @@ SVGSVGElement pathToSvgElement(SurfacePath path, SurfacePaintData paint) { (paint.style != ui.PaintingStyle.fill && paint.strokeWidth != 0 && paint.strokeWidth != null)) { - svgPath.setAttribute('stroke', colorValueToCssString(paint.color)!); + svgPath.setAttribute('stroke', colorValueToCssString(paint.color)); svgPath.setAttribute('stroke-width', '${paint.strokeWidth ?? 1.0}'); if (paint.strokeCap != null) { svgPath.setAttribute('stroke-linecap', '${stringForStrokeCap(paint.strokeCap)}'); } svgPath.setAttribute('fill', 'none'); } else { - svgPath.setAttribute('fill', colorValueToCssString(paint.color)!); + svgPath.setAttribute('fill', colorValueToCssString(paint.color)); } if (path.fillType == ui.PathFillType.evenOdd) { svgPath.setAttribute('fill-rule', 'evenodd'); diff --git a/lib/web_ui/lib/src/engine/html/painting.dart b/lib/web_ui/lib/src/engine/html/painting.dart index 78e5f64239f7c..1b573a933b38d 100644 --- a/lib/web_ui/lib/src/engine/html/painting.dart +++ b/lib/web_ui/lib/src/engine/html/painting.dart @@ -268,7 +268,7 @@ class SurfacePaintData { if (strokeJoin != null) { buffer.write('strokeJoin = $strokeJoin; '); } - buffer.write('color = ${colorToCssString(ui.Color(color))}; '); + buffer.write('color = ${ui.Color(color).toCssString()}; '); if (shader != null) { buffer.write('shader = $shader; '); } diff --git a/lib/web_ui/lib/src/engine/html/shaders/shader.dart b/lib/web_ui/lib/src/engine/html/shaders/shader.dart index effc2c6a5a96f..7907cc6009b9f 100644 --- a/lib/web_ui/lib/src/engine/html/shaders/shader.dart +++ b/lib/web_ui/lib/src/engine/html/shaders/shader.dart @@ -407,13 +407,13 @@ void _addColorStopsToCanvasGradient(DomCanvasGradient gradient, } if (colorStops == null) { assert(colors.length == 2); - gradient.addColorStop(offset, colorToCssString(colors[0])!); - gradient.addColorStop(1 - offset, colorToCssString(colors[1])!); + gradient.addColorStop(offset, colors[0].toCssString()); + gradient.addColorStop(1 - offset, colors[1].toCssString()); } else { for (int i = 0; i < colors.length; i++) { final double colorStop = colorStops[i].clamp(0.0, 1.0); gradient.addColorStop( - colorStop * scale + offset, colorToCssString(colors[i])!); + colorStop * scale + offset, colors[i].toCssString()); } } if (isDecal) { @@ -841,7 +841,7 @@ class ModeHtmlColorFilter extends EngineHtmlColorFilter { if (blendMode == ui.BlendMode.saturation || blendMode == ui.BlendMode.multiply || blendMode == ui.BlendMode.modulate) { - filterElement!.style.backgroundColor = colorToCssString(color)!; + filterElement!.style.backgroundColor = color.toCssString(); } return svgFilter.element; } diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 0b32f66661a4c..65f91fc12558e 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -791,13 +791,13 @@ void applyTextStyleToElement({ final double adaptedWidth = strokeWidth != null && strokeWidth > 0 ? strokeWidth : 1.0 / ui.window.devicePixelRatio; - cssStyle.textStroke = '${adaptedWidth}px ${colorToCssString(color)}'; + cssStyle.textStroke = '${adaptedWidth}px ${color?.toCssString()}'; } else if (color != null) { - cssStyle.color = colorToCssString(color)!; + cssStyle.color = color.toCssString(); } final ui.Color? background = style.background?.color; if (background != null) { - cssStyle.backgroundColor = colorToCssString(background)!; + cssStyle.backgroundColor = background.toCssString(); } final double? fontSize = style.fontSize; if (fontSize != null) { @@ -843,7 +843,7 @@ void applyTextStyleToElement({ } final ui.Color? decorationColor = style.decorationColor; if (decorationColor != null) { - cssStyle.textDecorationColor = colorToCssString(decorationColor)!; + cssStyle.textDecorationColor = decorationColor.toCssString(); } } } @@ -878,7 +878,7 @@ String _shadowListToCss(List shadows) { } final ui.Shadow shadow = shadows[i]; sb.write('${shadow.offset.dx}px ${shadow.offset.dy}px ' - '${shadow.blurRadius}px ${colorToCssString(shadow.color)}'); + '${shadow.blurRadius}px ${shadow.color.toCssString()}'); } return sb.toString(); } diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 0f81e9dcaea79..3b7eca17a535f 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -339,20 +339,15 @@ bool rectContainsOther(ui.Rect rect, ui.Rect other) { rect.bottom >= other.bottom; } -/// Converts color to a css compatible attribute value. -String? colorToCssString(ui.Color? color) { - if (color == null) { - return null; +extension CssColor on ui.Color { + /// Converts color to a css compatible attribute value. + String toCssString() { + return colorValueToCssString(value); } - final int value = color.value; - return colorValueToCssString(value); } // Converts a color value (as an int) into a CSS-compatible value. -String? colorValueToCssString(int? value) { - if (value == null) { - return null; - } +String colorValueToCssString(int value) { if (value == 0xFF000000) { return '#000000'; } @@ -690,7 +685,7 @@ void setThemeColor(ui.Color? color) { ..name = 'theme-color'; domDocument.head!.append(theme); } - theme.content = colorToCssString(color)!; + theme.content = color.toCssString(); } else { theme?.remove(); } diff --git a/lib/web_ui/test/html/path_to_svg_golden_test.dart b/lib/web_ui/test/html/path_to_svg_golden_test.dart index 0356f1f1d30b8..c40471289cf21 100644 --- a/lib/web_ui/test/html/path_to_svg_golden_test.dart +++ b/lib/web_ui/test/html/path_to_svg_golden_test.dart @@ -194,14 +194,14 @@ DomElement pathToSvgElement(Path path, Paint paint, bool enableFill) { root.append(pathElement); if (paint.style == PaintingStyle.stroke || paint.strokeWidth != 0.0) { - pathElement.setAttribute('stroke', colorToCssString(paint.color)!); + pathElement.setAttribute('stroke', paint.color.toCssString()); pathElement.setAttribute('stroke-width', paint.strokeWidth); if (!enableFill) { pathElement.setAttribute('fill', 'none'); } } if (paint.style == PaintingStyle.fill) { - pathElement.setAttribute('fill', colorToCssString(paint.color)!); + pathElement.setAttribute('fill', paint.color.toCssString()); } pathElement.setAttribute('d', pathToSvg((path as SurfacePath).pathRef)); // This is what we're testing! return root; From e64e9b8510da80eb8c8316304751231ddeb8e34c Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 3 May 2023 13:23:44 -0400 Subject: [PATCH 2/2] few more missing callsites after rebase --- .../application_switcher_description_test.dart | 8 ++++---- .../platform_dispatcher/system_ui_overlay_style_test.dart | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/web_ui/test/engine/platform_dispatcher/application_switcher_description_test.dart b/lib/web_ui/test/engine/platform_dispatcher/application_switcher_description_test.dart index 5d3238402abc1..1dd6902e6aa88 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/application_switcher_description_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/application_switcher_description_test.dart @@ -47,7 +47,7 @@ Future testMain() async { const ui.Color expectedPrimaryColor = ui.Color(0xFF00FF00); expect(domDocument.title, 'Title Test'); - expect(getCssThemeColor(), colorToCssString(expectedPrimaryColor)); + expect(getCssThemeColor(), expectedPrimaryColor.toCssString()); ui.window.sendPlatformMessage( 'flutter/platform', @@ -64,7 +64,7 @@ Future testMain() async { const ui.Color expectedNewPrimaryColor = ui.Color(0xFFFABADA); expect(domDocument.title, 'Different title'); - expect(getCssThemeColor(), colorToCssString(expectedNewPrimaryColor)); + expect(getCssThemeColor(), expectedNewPrimaryColor.toCssString()); }); test('supports null title and primaryColor', () { @@ -89,7 +89,7 @@ Future testMain() async { ); expect(domDocument.title, ''); - expect(getCssThemeColor(), colorToCssString(expectedNullColor)); + expect(getCssThemeColor(), expectedNullColor.toCssString()); domDocument.title = 'Something Else'; expect(domDocument.title, 'Something Else'); @@ -104,7 +104,7 @@ Future testMain() async { ); expect(domDocument.title, ''); - expect(getCssThemeColor(), colorToCssString(expectedNullColor)); + expect(getCssThemeColor(), expectedNullColor.toCssString()); }); }); } diff --git a/lib/web_ui/test/engine/platform_dispatcher/system_ui_overlay_style_test.dart b/lib/web_ui/test/engine/platform_dispatcher/system_ui_overlay_style_test.dart index b79a7b429dbe4..8e040bfc43828 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/system_ui_overlay_style_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/system_ui_overlay_style_test.dart @@ -44,7 +44,7 @@ void testMain() { const ui.Color statusBarColor = ui.Color(0xFFF44336); sendSetSystemUIOverlayStyle(statusBarColor: statusBarColor); - expect(getCssThemeColor(), colorToCssString(statusBarColor)); + expect(getCssThemeColor(), statusBarColor.toCssString()); sendSetSystemUIOverlayStyle(); expect(getCssThemeColor(), null);