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

Conversation

@flar
Copy link
Contributor

@flar flar commented Feb 26, 2022

We have encountered some crashes downstream when Flutter apps create ColorFilter objects that are essentially a NOP operation. Internally in the new DlColorFilter code we missed cases where the skia_object() version of the filter might return null. This PR fixes a couple of situations with respect to these NOP filters:

  • First, the internal uses of skia_object() check for null where appropriate
  • Then, we avoid propagating instances of DlColorFilter where the skia analog is null to avoid the churn when downstream code gets a DlColorFilter and that kicks off tests, but the filter is a NOP.
  • Finally, similar code protects propagation of DlMaskFilter objects, but they had no cases where the code might try to dereference the skia version.


import 'package:litetest/litetest.dart';

const Color transparent = Color(0x00000000);
Copy link
Contributor

@dnfield dnfield Feb 26, 2022

Choose a reason for hiding this comment

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

Ubernit: transparentBlack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's a test failure I have to amend it for...

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

Lgtm

@flar flar added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 26, 2022
@flar flar merged commit 729f5b2 into flutter:main Feb 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants