-
Notifications
You must be signed in to change notification settings - Fork 6k
Simplify FlView #39467
Simplify FlView #39467
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 (don't just cc him here, he won't see it! He's on Discord!). 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. |
|
@cbracken @gspencergoog do you know how to get a Linux app to have multiple layers so I can confirm that still renders and will render with the proposed single FlGLArea change? |
|
@jpnurmi I don't think this change will specifically fix the issue you were seeing, but can you test in case it does? |
|
I could still see a blank window occasionally slip through with this but not with #39473. |
|
For what it's worth, this PR, along with adding in the main function fixes the flickering that I have been experiencing in flutter/flutter#120254 |
cbracken
left a comment
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.
This PR simplifies the FlView GL interactions. It removes some code paths that were never being used (possibly for future use with platform views?) and complex internal APIs that could be done in a single method call. This is motivated in investigating https://github.com/ubuntu-flutter-community/software/issues/868. I plan to take this further, since there doesn't seem to be any reason why multiple FlGLArea widgets are used - we can draw all the layers in one widget. For reviewers - the individual commits aren't particularly interesting, I just did it that way to be sure I wasn't breaking anything along the way and help to diagnose any regressions in the future. The structural changes that this PR is simplifying came in f94cf14.
This reverts commit f37f48d.

This PR simplifies the FlView GL interactions. It removes some code paths that were never being used (possibly for future use with platform views?) and complex internal APIs that could be done in a single method call. This is motivated in investigating https://github.com/ubuntu-flutter-community/software/issues/868.
I plan to take this further, since there doesn't seem to be any reason why multiple FlGLArea widgets are used - we can draw all the layers in one widget.
For reviewers - the individual commits aren't particularly interesting, I just did it that way to be sure I wasn't breaking anything along the way and help to diagnose any regressions in the future.
The structural changes that this PR is simplifying came in f94cf14.
Pre-launch Checklist
///).