-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] Fix webkit ColorFilter.mode for webkit #27361
Conversation
| final ui.ColorFilter filter; | ||
| html.Element? _filterElement; | ||
| bool containerVisible = true; | ||
| static int activeColorFilterCount = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a static, can we have a PrerollContext class that's passed down to the preroll method recursively? This class could have a (non-static) field activeColorFilterCount.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| ++activeColorFilterCount; | ||
| super.preroll(); | ||
| --activeColorFilterCount; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example with PrerollContext:
void preroll(PrerollContext context) {
context.activeColorFilterCount++;
super.preroll(context);
context.activeColorFilterCount--;
}We can later add other things to the context, such as transform and clip stack, and avoid doing parent look-ups that we do today in a few places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
|
||
| /// Add an element as a global resource to be referenced by CSS. | ||
| /// | ||
| /// This call create a global resource host element on demand and either |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "This method creates"
| html.Element? _sceneHostElement; | ||
|
|
||
| /// A child element of body outside the shadowroot that hosts | ||
| /// global resources such svg filters and clip paths when using webkit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to code in addResource this element hosts resources on all browsers, not only when using webkit, and it not always a child of the body element.
| _resourcesHost = html.DivElement() | ||
| ..style.visibility = 'hidden'; | ||
| if (isWebKit) { | ||
| final html.Node bodyNode = html.document.body!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it easier for us to support multiple Flutter views in the future, can we add this element to flt-glass-pane instead of body?
1- Marks subtree of colorfilter (css filter attribute with svg filter) to prevent canvas use due to webkit bug that skips applying filter to canvas element in subtree. Same fix as prior ShaderMask failure in webkit.
2- Moves svg element before the filter interior div since Safari sometimes doesn't apply filter if it is defined after the element that contains css attribute.
3- WebKit fails to resolve filter: url("#fcf") when svg is referenced inside shadowdom. This PR moves to a svg resource model that adds/removes filters to a root level host node to get around webkit failure.
Fixes: flutter/flutter#85733
Pre-launch Checklist
writing and running engine tests.
///).