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.

Sign up for GitHub

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

Revert "[web] Fix image gap due to svg element without position attribute" #21986

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Revert "[web] Fix image gap due to svg element without position attribute" #21986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions lib/web_ui/lib/src/engine/html/color_filter.dart
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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);
}