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

Conversation

@harryterkelsen
Copy link
Contributor

@harryterkelsen harryterkelsen commented Dec 3, 2020

Description

Implements pushColorFilter in ui.SceneBuilder in CanvasKit.

Related Issues

Fixes the following issues in CanvasKit mode:

Fixes flutter/flutter#71733

Tests

I added the following tests:

Added a test to scene_test.dart

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

@harryterkelsen
Copy link
Contributor Author

I could also wait for the SkPicture resurrection PR to land and add a golden test for this.

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

});
// TODO: https://github.com/flutter/flutter/issues/60040

test('pushColorFilter does not throw', () async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Bonus points for making it a screenshot test. To do that, simply rename the file to scene_golden_test.dart and use golden_tester.dart like normal (example).

Copy link
Contributor

Choose a reason for hiding this comment

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

although the test would need to be skipped outside chrome.

@chinmaygarde chinmaygarde added the platform-web Code specifically for the web engine label Dec 10, 2020
@chinmaygarde
Copy link
Member

Can we land this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ColorFiltered UnimplementedError with CanvasKit

3 participants