Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@ferhatb
Copy link
Contributor

@ferhatb ferhatb commented May 3, 2020

Related issues: flutter/flutter#56071 , flutter/flutter#50897

Verified by comparing native vs web. Golden tests results: flutter/goldens#92

Screen Shot 2020-05-03 at 11 17 59 AM

Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

ui.Image image, ui.Offset p, SurfacePaintData paint) {
final HtmlImage htmlImage = image;
final html.Element imgElement = htmlImage.cloneImageElement();
final ui.BlendMode blendMode = paint.blendMode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this line should move down, closer to the location where it's used

}
}

const String _flutterLogoBase64 =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document (or put in the variable name) the size of the test image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(swapLayers
? '<feBlend in="SourceGraphic" in2="flood" mode="$feBlend"/>'
: '<feBlend in="flood" in2="SourceGraphic" mode="$feBlend"/>') +
'</filter></svg>';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These SVG snippets contain a lot of static content that need to be parsed from HTML. Wouldn't using the SVG API be more efficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to pull in svg library, not as big as dart:html but not small.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something tells me we already have it all included :)

It's OK. We can profile it later and see what the size/speed trade-off is like.

@ferhatb
Copy link
Contributor Author

ferhatb commented May 5, 2020

@ferhatb ferhatb merged commit a0983d3 into flutter:master May 5, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 6, 2020
GaryQian pushed a commit to flutter/flutter that referenced this pull request May 6, 2020
* c2c6ad9 Roll src/third_party/skia 41e377d1baf0..3d311a983bf0 (9 commits) (flutter/engine#18150)

* a0983d3 [web] Add support for ColorFilter on images (flutter/engine#18111)

* 905836d [web] Upgrade the test package to remove the dependency on package_resolver (flutter/engine#18135)

* 0645e93 Manually add dependency for shelf_proxy (flutter/engine#18151)

* d104f87 Roll src/third_party/skia 3d311a983bf0..0cbd58766ace (2 commits) (flutter/engine#18152)

* b1b1df0 Roll fuchsia/sdk/core/mac-amd64 from ERGnT... to c5NQZ... (flutter/engine#18155)

* 8a8cd5d Roll src/third_party/dart 7bb38670d279..1ef444139c4c (22 commits) (flutter/engine#18161)

* c1068de gpu_metal_surface `submitframe` return false when canvas is null (flutter/engine#18154)

* cf13c7f [iOS platform view] fix active_composition_order_ never cleared (flutter/engine#18153)

* 805a887 Add first Linux shell tests (flutter/engine#18159)

* 80c090d Fix path bounds for multiple rects. Implement winding rules (flutter/engine#18165)

* 15752ee Roll src/third_party/skia 0cbd58766ace..e1c0cb3de8ab (8 commits) (flutter/engine#18167)

* c1bd64a Roll src/third_party/skia e1c0cb3de8ab..1e8fb04b29b9 (3 commits) (flutter/engine#18168)

* 6a379cc Roll fuchsia/sdk/core/linux-amd64 from gnNdl... to RpHTv... (flutter/engine#18171)

* 961fa14 Roll src/third_party/dart 1ef444139c4c..8c8249fa0123 (11 commits) (flutter/engine#18172)

* 337ec63 Roll src/third_party/skia 1e8fb04b29b9..c5727d8e34d2 (1 commits) (flutter/engine#18173)

* bc30355 Roll src/third_party/skia c5727d8e34d2..999257d870d7 (5 commits) (flutter/engine#18174)

* c3cd83b Roll fuchsia/sdk/core/mac-amd64 from c5NQZ... to jMJqf... (flutter/engine#18175)

* Updated bin/internal/fuchsia-linux.version

* Updated bin/internal/fuchsia-mac.version
@ferhatb ferhatb deleted the colorfilter branch May 8, 2020 19:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants