-
Notifications
You must be signed in to change notification settings - Fork 24.8k
feat(iOS) - blur filter using SwiftUI #52495
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.
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
base: main
Are you sure you want to change the base?
feat(iOS) - blur filter using SwiftUI #52495
Conversation
wip somewhat working working fix additional stuff update layout metrics update blur radius init working revert use layer reset dkd fix final version working working grayscale parent frame set swiftui container wip nit nit enable blur filter example cleanup renaming cleanup naming remove unnecessary configs nits wip nit child mount working remove needs swiftui container flag working nit nit nit remove all animations remove all animations nit working refactor nit nit fix brightness filter overlay nit
Visual demo and additional context for This Week in React readers 😉 |
Thanks for continuing to push us forward, @intergalacticspacehighway. |
@intergalacticspacehighway are you working directly with somebody in the team to have this pushed forward?
Those kind of changes should usually go through some kind of process, like RFC or similar, with somebody at Meta championing it. Otherwise they are very hard to land. |
@cipolleschi i have been chatting with @NickGerleman regarding SwiftUI here, also we were having some conversation over discord. The intention of the PR was not to introduce official SwiftUI support, but it is more of an implementation detail. It is touching 2 core files, others are completely new files or small configs. I will look into SwiftPM migration, but i wanted to discuss the approach before investing too much time into it and Nick has some context on it, also I tagged @chrfalch for SwiftPM concern. Will make sure to do an RFC next time! 🙏 |
I will try to take a look at this next week. This week has been an eventful oncall shift, so I am a bit backed up atm. |
Sorry for the delay. Had a chance to go through all of this! @joevilches will be a lot more familiar with how container view is set up. I know there was some intricacy, around how we set things up, depending on what we do with borders. I don't know if there is any way we can avoid the complexity with A couple interesting scenarios came to mind, that would be helpful to validate explicitly:
The other bit, around complexity, is that we now have a "filter layer" with logic only ever used for brightness, where we get creative with blend modes to implement that. But the SwiftUI approach is really generic, and as you mention, we could use the SwifUI filter for that. Afaict, SwiftUI has built in support for These all seem supported on iOS 13+ too, so we could fully support all the filters on iOS. Concrete Steps: There are a couple interesting pieces of this. Can we split them up? E.g.
|
Thank you for reviewing @NickGerleman!
We need to remove the visual styles from parent view's layer or else they blend and make blur on child look weird. So i thought simplest way would be to copy the visual styles to SwiftUI content view layer. I also tried making a visual only layer and then just togging them between SwiftUI container and main container when blur gets added/removed. But this was adding more complexity.
Yes, it mimics the existing container approach, so cleans it up when prop is removed.
Yeah, there is. It should be fixed once we merge this - #52413. Will add example in hit testing when blur/SwiftUI view is on.
The usecase for container view iirc was overflow + boxShadow. i.e. even when overflow is hidden, boxShadow still needs to be out of bounds. This works with the blur since the container becomes child of the SwiftUI content view. Will add example to cover this.
Yes! Once we finalize the approach, will do a separate PR to add them and replace the brightness filter and also the feature flag. |
Thanks! This is so so awesome. I spent many days racking my brain trying to get this to work last year. Super excited to see you push through here! I think the general approach is good, but I am trying to think of ways to reduce complexity, since the Some ideas
|
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.
requesting changes for some of the things Nick asked for (like the feature flag), along with some of my nits, and an attempt to encapsulate some of the 2-view logic that is going on
@@ -63,7 +63,7 @@ void ViewShadowNode::initialize() noexcept { | |||
viewProps.mixBlendMode != BlendMode::Normal || | |||
viewProps.isolation == Isolation::Isolate || | |||
HostPlatformViewTraitsInitializer::formsStackingContext(viewProps) || | |||
!viewProps.accessibilityOrder.empty(); | |||
!viewProps.accessibilityOrder.empty() || !viewProps.filter.empty(); |
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.
we have this already on line 62
|
||
- (UIView *)contentView; | ||
- (void)updateBlurRadius:(NSNumber *)radius; | ||
- (void)updateGrayScale:(NSNumber *)amount; |
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.
Doesn't seem like we use this. Can we either get rid of it here and add it later in another PR, or change RCTViewComponentView code to use?
@@ -793,44 +802,88 @@ - (BOOL)styleWouldClipOverflowInk | |||
((!_props->boxShadow.empty() || (clipToPaddingBox && nonZeroBorderWidth)) || _props->outlineWidth != 0); | |||
} | |||
|
|||
- (UIView *)childContainerView |
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.
name is kinda confusing, so is mine below. Can you be specific? This is only used for filter
maybe change it to filterView
or something
_swiftUIWrapper.hostingView.frame = CGRectMake(0, 0, self.bounds.size.width, self.bounds.size.height); | ||
UIView *swiftUIContentView = | ||
[[UIView alloc] initWithFrame:CGRectMake(0, 0, self.bounds.size.width, self.bounds.size.height)]; |
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.
seems like the wrapper can handle this and we can encapsulate these 2 calls inside and just call some function the wrapper exposes that sets the frame/inits the content view
if (_swiftUIWrapper) { | ||
_swiftUIWrapper.hostingView.frame = self.bounds; | ||
if (_swiftUIWrapper.contentView) { | ||
_swiftUIWrapper.contentView.frame = self.bounds; | ||
} | ||
} |
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.
Can we encapsulate layout updates into the wrapper?
oh sorry one more thing - can we test this with some combo of clipping and overflow ink (box shadow, outline, etc). As an aside: we should add an "amalgamation" example in RNTester that just adds every style to one unfortunate |
Can we add an example for applying blurRadius with animation as well? cc: @joevilches |
Thank you @joevilches 🙏. Will make the above mentioned changes and update the PR.
This would be really amazing. We can make |
👀 |
Animated.timing(animatedValue, { | ||
toValue: isBlurred ? 0 : 20, | ||
duration: 1000, | ||
useNativeDriver: false, |
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.
For native driver support we need to merge this - #52920
Summary:
As per the discussion on the previous PR thread, this PR uses
SwiftUI
to implement blur filter on iOS.Approach:
To implement blur filter on iOS, we have two options:
Use
CAFilter
(private API, app can get rejected/API can break). Earlier PR was using that approach. Thanks to Nick for suggesting SwiftUI API.Use
SwiftUI
. Wrap the view in a SwiftUI view and apply blur. This PR builds on top of that approach. This also enables a way to addSwiftUI
only features like this one. Additional filters (grayscale, saturate, contrast, hueRotate) can also be added.There are a few ways we can implement the SwiftUI approach:
RCTSwiftUIComponentView
-> do style flattening in View -> check iffilter
is present and conditionally render theRCTSwiftUIComponentView
on iOS, wrap children with aSwiftUI
view. Tradeoff with this approach is that it addsStyleSheet.flatten
overhead on JS side.SwiftUI
container view inside ofRCTViewComponentView
. Tradeoff with this approach is that it complicatesRCTViewComponentView
a bit.I decided to go with 2 to avoid the flattening tradeoff and try to minimize complicating
RCTViewComponentView
. it only adds the wrapper if it's required and removes if not (in this PR, blur filter style will add the wrapper, it will get removed if blur filter styling gets removed). It uses the existing container view pattern.Changelog:
[IOS][ADDED] - Filter blur
Test Plan:
Test filter blur example on iOS. SwiftUI view should be added to the hierarchy.
Aside:
SwiftUI
's brightness, which would be cleaner imo (tested and it works).