[iOS] Fixes platform view clipped when FlutterView has non-zero offset #24893
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
cc @xster for review. |
There was a problem hiding this comment.
This will not handle when PlatformViews draw outside of the ChildClippingView.
Is flutter/flutter#66968 caused by flutter_view_ is nil?
I see, the FlutterView's offset is (50, 50). Are we trying to to display PlatformView outside of the flutter view?
|
I think the issue might not be with the maskView, but rather the frame of the platform view. |
@cyanglaz Seems platform view's frame is correct, reverse offset of the |
There was a problem hiding this comment.
The change looks good. It seems to be that it should produce the same result as
CGRect maskViewFrame = [flutter_view_ convertRect:flutter_view_.get().frame toView:clipView];
Could you explain why in this add to app scenario, it is different?
Can we add a test for this? Possibly a scenario test.
@cyanglaz I think the difference is we don't have to take flutter_view_'s frame origin x/y into account.
I saw the scenario test code, seems it should not depend on flutter framework https://github.com/flutter/engine/blob/5d9509ae05/testing/scenario_app/README.md#scenario-app . I may need your help for adding test :) (Seems we need Flutter framework to reproduce it, use dart:ui directly can't reproduce) |
|
@zhongwuzw Thanks. The fix looks good to me. As for the test, I don't think we need the framework to change the flutterView's frame. (If changing flutterView's frame is the way to reproduce the bug) Here we have access to the FlutterViewController: https://github.com/flutter/engine/blob/master/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m#L98-L105 |
|
@cyanglaz Ahha, my mistake, I don't clean the Xcode Derived cache, think we should need Flutter framework wrongly, I added the scenario test. :) |
Fixes flutter/flutter#66968
cc. @cyanglaz .
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.