Skip to content

Commit

Permalink
Revert "[web] Fix image gap due to svg element without position attri…
Browse files Browse the repository at this point in the history 
…bute (flutter#21939)" (flutter#21986)

This reverts commit 7987980.
  • Loading branch information
@zanderso
zanderso authored Oct 20, 2020
1 parent 326157e commit 2ef0f42
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 146 deletions.
16 changes: 7 additions & 9 deletions lib/web_ui/lib/src/engine/html/color_filter.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,7 @@ String? svgFilterFromBlendMode(
case ui.BlendMode.dstOver:
case ui.BlendMode.clear:
case ui.BlendMode.srcOver:
assert(
false,
'Invalid svg filter request for blend-mode '
assert(false, 'Invalid svg filter request for blend-mode '
'$colorFilterBlendMode');
break;
}
Expand All @@ -234,7 +232,7 @@ int _filterIdCounter = 0;
// A' = a1*R + a2*G + a3*B + a4*A + a5
String _srcInColorFilterToSvg(ui.Color? color) {
_filterIdCounter += 1;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" '
'filterUnits="objectBoundingBox" x="0%" y="0%" width="100%" height="100%">'
'<feColorMatrix values="0 0 0 0 1 ' // Ignore input, set it to absolute.
Expand All @@ -251,7 +249,7 @@ String _srcInColorFilterToSvg(ui.Color? color) {

String _srcOutColorFilterToSvg(ui.Color? color) {
_filterIdCounter += 1;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" '
'filterUnits="objectBoundingBox" x="0%" y="0%" width="100%" height="100%">'
'<feFlood flood-color="${colorToCssString(color)}" flood-opacity="1" result="flood">'
Expand All @@ -263,7 +261,7 @@ String _srcOutColorFilterToSvg(ui.Color? color) {

String _xorColorFilterToSvg(ui.Color? color) {
_filterIdCounter += 1;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" '
'filterUnits="objectBoundingBox" x="0%" y="0%" width="100%" height="100%">'
'<feFlood flood-color="${colorToCssString(color)}" flood-opacity="1" result="flood">'
Expand All @@ -278,7 +276,7 @@ String _xorColorFilterToSvg(ui.Color? color) {
String _compositeColorFilterToSvg(
ui.Color? color, double k1, double k2, double k3, double k4) {
_filterIdCounter += 1;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" '
'filterUnits="objectBoundingBox" x="0%" y="0%" width="100%" height="100%">'
'<feFlood flood-color="${colorToCssString(color)}" flood-opacity="1" result="flood">'
Expand All @@ -297,7 +295,7 @@ String _modulateColorFilterToSvg(ui.Color color) {
final double r = color.red / 255.0;
final double b = color.blue / 255.0;
final double g = color.green / 255.0;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" '
'filterUnits="objectBoundingBox" x="0%" y="0%" width="100%" height="100%">'
'<feColorMatrix values="0 0 0 0 $r ' // Ignore input, set it to absolute.
Expand All @@ -314,7 +312,7 @@ String _modulateColorFilterToSvg(ui.Color color) {
String _blendColorFilterToSvg(ui.Color? color, String? feBlend,
{bool swapLayers = false}) {
_filterIdCounter += 1;
return '$kSvgResourceHeader'
return '<svg width="0" height="0">'
'<filter id="_fcf$_filterIdCounter" filterUnits="objectBoundingBox" '
'x="0%" y="0%" width="100%" height="100%">'
'<feFlood flood-color="${colorToCssString(color)}" flood-opacity="1" result="flood">'
Expand Down
86 changes: 34 additions & 52 deletions lib/web_ui/lib/src/engine/util.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ TransformKind transformKindOf(Float32List matrix) {

// If matrix contains scaling, rotation, z translation or
// perspective transform, it is not considered simple.
final bool isSimple2dTransform = m[15] ==
1.0 && // start reading from the last element to eliminate range checks in subsequent reads.
final bool isSimple2dTransform =
m[15] == 1.0 && // start reading from the last element to eliminate range checks in subsequent reads.
m[14] == 0.0 && // z translation is NOT simple
// m[13] - y translation is simple
// m[12] - x translation is simple
Expand All @@ -126,8 +126,8 @@ TransformKind transformKindOf(Float32List matrix) {
// m[4] - 2D rotation is simple
m[3] == 0.0 &&
m[2] == 0.0;
// m[1] - 2D rotation is simple
// m[0] - scale x is simple
// m[1] - 2D rotation is simple
// m[0] - scale x is simple

if (!isSimple2dTransform) {
return TransformKind.complex;
Expand All @@ -136,7 +136,8 @@ TransformKind transformKindOf(Float32List matrix) {
// From this point on we're sure the transform is 2D, but we don't know if
// it's identity or not. To check, we need to look at the remaining elements
// that were not checked above.
final bool isIdentityTransform = m[0] == 1.0 &&
final bool isIdentityTransform =
m[0] == 1.0 &&
m[1] == 0.0 &&
m[4] == 0.0 &&
m[5] == 1.0 &&
Expand Down Expand Up @@ -164,7 +165,7 @@ bool isIdentityFloat32ListTransform(Float32List matrix) {
/// transform. Consider removing the CSS `transform` property from elements
/// that apply identity transform.
String float64ListToCssTransform2d(Float32List matrix) {
assert(transformKindOf(matrix) != TransformKind.complex);
assert (transformKindOf(matrix) != TransformKind.complex);
return 'matrix(${matrix[0]},${matrix[1]},${matrix[4]},${matrix[5]},${matrix[12]},${matrix[13]})';
}

Expand Down Expand Up @@ -278,22 +279,10 @@ void transformLTRB(Matrix4 transform, Float32List ltrb) {

_tempPointMatrix.multiplyTranspose(transform);

ltrb[0] = math.min(
math.min(
math.min(_tempPointData[0], _tempPointData[1]), _tempPointData[2]),
_tempPointData[3]);
ltrb[1] = math.min(
math.min(
math.min(_tempPointData[4], _tempPointData[5]), _tempPointData[6]),
_tempPointData[7]);
ltrb[2] = math.max(
math.max(
math.max(_tempPointData[0], _tempPointData[1]), _tempPointData[2]),
_tempPointData[3]);
ltrb[3] = math.max(
math.max(
math.max(_tempPointData[4], _tempPointData[5]), _tempPointData[6]),
_tempPointData[7]);
ltrb[0] = math.min(math.min(math.min(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), _tempPointData[3]);
ltrb[1] = math.min(math.min(math.min(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), _tempPointData[7]);
ltrb[2] = math.max(math.max(math.max(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), _tempPointData[3]);
ltrb[3] = math.max(math.max(math.max(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), _tempPointData[7]);
}

/// Returns true if [rect] contains every point that is also contained by the
Expand All @@ -311,13 +300,6 @@ bool rectContainsOther(ui.Rect rect, ui.Rect other) {
/// Counter used for generating clip path id inside an svg <defs> tag.
int _clipIdCounter = 0;

/// Used for clipping and filter svg resources.
///
/// Position needs to be absolute since these svgs are sandwiched between
/// canvas elements and can cause layout shifts otherwise.
const String kSvgResourceHeader = '<svg width="0" height="0" '
'style="position:absolute">';

/// Converts Path to svg element that contains a clip-path definition.
///
/// Calling this method updates [_clipIdCounter]. The HTML id of the generated
Expand All @@ -329,7 +311,8 @@ String _pathToSvgClipPath(ui.Path path,
double scaleY = 1.0}) {
_clipIdCounter += 1;
final StringBuffer sb = StringBuffer();
sb.write(kSvgResourceHeader);
sb.write('<svg width="0" height="0" '
'style="position:absolute">');
sb.write('<defs>');

final String clipId = 'svgClip$_clipIdCounter';
Expand Down Expand Up @@ -437,11 +420,10 @@ const Set<String> _genericFontFamilies = <String>{
///
/// For iOS, default to -apple-system, where it should be available, otherwise
/// default to Arial. BlinkMacSystemFont is used for Chrome on iOS.
final String _fallbackFontFamily =
_isMacOrIOS ? '-apple-system, BlinkMacSystemFont' : 'Arial';
final String _fallbackFontFamily = _isMacOrIOS ?
'-apple-system, BlinkMacSystemFont' : 'Arial';

bool get _isMacOrIOS =>
operatingSystem == OperatingSystem.iOs ||
bool get _isMacOrIOS => operatingSystem == OperatingSystem.iOs ||
operatingSystem == OperatingSystem.macOs;

/// Create a font-family string appropriate for CSS.
Expand All @@ -457,10 +439,8 @@ String? canonicalizeFontFamily(String? fontFamily) {
// on sans-serif.
// Map to San Francisco Text/Display fonts, use -apple-system,
// BlinkMacSystemFont.
if (fontFamily == '.SF Pro Text' ||
fontFamily == '.SF Pro Display' ||
fontFamily == '.SF UI Text' ||
fontFamily == '.SF UI Display') {
if (fontFamily == '.SF Pro Text' || fontFamily == '.SF Pro Display' ||
fontFamily == '.SF UI Text' || fontFamily == '.SF UI Display') {
return _fallbackFontFamily;
}
}
Expand Down Expand Up @@ -494,27 +474,27 @@ void applyWebkitClipFix(html.Element? containerElement) {
}
}

final ByteData? _fontChangeMessage =
JSONMessageCodec().encodeMessage(<String, dynamic>{'type': 'fontsChange'});
final ByteData? _fontChangeMessage = JSONMessageCodec().encodeMessage(<String, dynamic>{'type': 'fontsChange'});

// Font load callbacks will typically arrive in sequence, we want to prevent
// sendFontChangeMessage of causing multiple synchronous rebuilds.
// This flag ensures we properly schedule a single call to framework.
bool _fontChangeScheduled = false;

FutureOr<void> sendFontChangeMessage() async {
if (window._onPlatformMessage != null && !_fontChangeScheduled) {
_fontChangeScheduled = true;
// Batch updates into next animationframe.
html.window.requestAnimationFrame((num _) {
_fontChangeScheduled = false;
window.invokeOnPlatformMessage(
'flutter/system',
_fontChangeMessage,
(_) {},
);
});
}
if (window._onPlatformMessage != null)
if (!_fontChangeScheduled) {
_fontChangeScheduled = true;
// Batch updates into next animationframe.
html.window.requestAnimationFrame((num _) {
_fontChangeScheduled = false;
window.invokeOnPlatformMessage(
'flutter/system',
_fontChangeMessage,
(_) {},
);
});
}
}

// Stores matrix in a form that allows zero allocation transforms.
Expand Down Expand Up @@ -555,12 +535,14 @@ bool isUnsoundNull(dynamic object) {
}

bool _offsetIsValid(ui.Offset offset) {
assert(offset != null, 'Offset argument was null.'); // ignore: unnecessary_null_comparison
assert(!offset.dx.isNaN && !offset.dy.isNaN,
'Offset argument contained a NaN value.');
return true;
}

bool _matrix4IsValid(Float32List matrix4) {
assert(matrix4 != null, 'Matrix4 argument was null.'); // ignore: unnecessary_null_comparison
assert(matrix4.length == 16, 'Matrix4 must have 16 entries.');
return true;
}
Expand Down
106 changes: 21 additions & 85 deletions lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.10
// @dart = 2.6
import 'dart:html' as html;
import 'dart:js_util' as js_util;

import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -36,118 +35,55 @@ void testMain() async {
final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
final Picture backgroundPicture = _drawBackground();
builder.addPicture(Offset.zero, backgroundPicture);
builder.pushColorFilter(
EngineColorFilter.mode(Color(0xF0000080), BlendMode.color));
builder.pushColorFilter(EngineColorFilter.mode(Color(0xF0000080),
BlendMode.color));
final Picture circles1 = _drawTestPictureWithCircles(30, 30);
builder.addPicture(Offset.zero, circles1);
builder.pop();
html.document.body!.append(builder.build().webOnlyRootElement!);
html.document.body.append(builder
.build()
.webOnlyRootElement);

await matchGoldenFile('color_filter_blendMode_color.png', region: region);
});

/// Regression test for https://github.com/flutter/flutter/issues/59451.
///
/// Picture with overlay blend inside a physical shape. Should show image
/// at 0,0. In the filed issue it was leaving a gap on top.
test('Should render image with color filter without gap', () async {
final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
final Path path = Path();
path.addRRect(RRect.fromRectAndRadius(
Rect.fromLTRB(0, 0, 400, 400), Radius.circular(2)));
PhysicalShapeEngineLayer oldLayer = builder.pushPhysicalShape(
path: path, color: Color(0xFFFFFFFF), elevation: 0);
final Picture circles1 = _drawTestPictureWithImage(
ColorFilter.mode(Color(0x3C4043), BlendMode.overlay));
builder.addPicture(Offset(10, 0), circles1);
builder.pop();
builder.build();

final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder();
builder2.pushPhysicalShape(
path: path, color: Color(0xFFFFFFFF), elevation: 0, oldLayer: oldLayer);
builder2.addPicture(Offset(10, 0), circles1);
builder2.pop();

html.document.body!.append(builder2.build().webOnlyRootElement!);

await matchGoldenFile('color_filter_blendMode_overlay.png',
region: region);
});
}

Picture _drawTestPictureWithCircles(double offsetX, double offsetY) {
final EnginePictureRecorder recorder =
PictureRecorder() as EnginePictureRecorder;
final EnginePictureRecorder recorder = PictureRecorder();
final RecordingCanvas canvas =
recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400));
canvas.drawCircle(Offset(offsetX + 10, offsetY + 10), 10,
(Paint()..style = PaintingStyle.fill) as SurfacePaint);
recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400));
canvas.drawCircle(
Offset(offsetX + 10, offsetY + 10), 10, Paint()..style = PaintingStyle.fill);
canvas.drawCircle(
Offset(offsetX + 60, offsetY + 10),
10,
(Paint()
Paint()
..style = PaintingStyle.fill
..color = const Color.fromRGBO(255, 0, 0, 1)) as SurfacePaint);
..color = const Color.fromRGBO(255, 0, 0, 1));
canvas.drawCircle(
Offset(offsetX + 10, offsetY + 60),
10,
(Paint()
Paint()
..style = PaintingStyle.fill
..color = const Color.fromRGBO(0, 255, 0, 1)) as SurfacePaint);
..color = const Color.fromRGBO(0, 255, 0, 1));
canvas.drawCircle(
Offset(offsetX + 60, offsetY + 60),
10,
(Paint()
Paint()
..style = PaintingStyle.fill
..color = const Color.fromRGBO(0, 0, 255, 1)) as SurfacePaint);
return recorder.endRecording();
}

Picture _drawTestPictureWithImage(ColorFilter filter) {
final EnginePictureRecorder recorder =
PictureRecorder() as EnginePictureRecorder;
final RecordingCanvas canvas =
recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400));
final Image testImage = createTestImage();
canvas.drawImageRect(
testImage,
Rect.fromLTWH(0, 0, 200, 150),
Rect.fromLTWH(0, 0, 300, 300),
(Paint()
..style = PaintingStyle.fill
..colorFilter = filter
..color = const Color.fromRGBO(0, 0, 255, 1)) as SurfacePaint);
..color = const Color.fromRGBO(0, 0, 255, 1));
return recorder.endRecording();
}

Picture _drawBackground() {
final EnginePictureRecorder recorder =
PictureRecorder() as EnginePictureRecorder;
final EnginePictureRecorder recorder = PictureRecorder();
final RecordingCanvas canvas =
recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400));
recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400));
canvas.drawRect(
Rect.fromLTWH(8, 8, 400.0 - 16, 400.0 - 16),
(Paint()
Paint()
..style = PaintingStyle.fill
..color = Color(0xFFE0FFE0)) as SurfacePaint);
..color = Color(0xFFE0FFE0)
);
return recorder.endRecording();
}

HtmlImage createTestImage({int width = 200, int height = 150}) {
html.CanvasElement canvas =
new html.CanvasElement(width: width, height: height);
html.CanvasRenderingContext2D ctx = canvas.context2D;
ctx.fillStyle = '#E04040';
ctx.fillRect(0, 0, width / 3, height);
ctx.fill();
ctx.fillStyle = '#40E080';
ctx.fillRect(width / 3, 0, width / 3, height);
ctx.fill();
ctx.fillStyle = '#2040E0';
ctx.fillRect(2 * width / 3, 0, width / 3, height);
ctx.fill();
html.ImageElement imageElement = html.ImageElement();
imageElement.src = js_util.callMethod(canvas, 'toDataURL', <dynamic>[]);
return HtmlImage(imageElement, width, height);
}

0 comments on commit 2ef0f42

Please sign in to comment.