-
Notifications
You must be signed in to change notification settings - Fork 6k
[macOS] Implement unobstructed platform views #42960
Conversation
5c12795 to
b98df6f
Compare
734e869 to
05a74f4
Compare
|
The ABI remains stable but the API breaks. I'd rather not break. You could
use typedefs.
…On Wed, Jun 21, 2023 at 11:16 AM Loïc Sharma ***@***.***> wrote:
Note that this is identical to struct FlutterDamage with more generic
naming...
Are we allowed to rename structs in the embedder API? The embedder API
allows renaming members
<https://github.com/flutter/engine/blob/08aaa88bf67fcd5a18d4309d89c1e940a544e49a/shell/platform/embedder/embedder.h#L39-L41>,
I *think* renaming a struct wouldn't break ABI.
—
Reply to this email directly, view it on GitHub
<#42960 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAKYNJBPH3QUX5CYYXR7PDXMM26ZANCNFSM6AAAAAAZKOXYUM>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
shell/platform/embedder/embedder.h
Outdated
| /// The size of this struct. Must be sizeof(FlutterRegion). | ||
| size_t struct_size; | ||
| /// Number of rectangles in the region. | ||
| size_t num_rects; |
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've been inconsistent, but I believe the preferred naming pattern is rects_count
|
@chinmaygarde I deleted my comment as I figured it'd better to keep |
|
01d28c5 to
49665b8
Compare
1776db3 to
24491a1
Compare
|
@knopp FYI, Chris Bracken is out on vacation until July 10th and will likely not be able to review until then. Apologies for the delay! |
Thanks for the heads-up! |
|
I'm back and taking a look at this and the others! Thanks so much for sending them! |
24491a1 to
cf5f6d5
Compare
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.
Overall looks great - I'm still looking at the embedder API side changes. Sent the rest of the comments.
shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm
Outdated
Show resolved
Hide resolved
cf5f6d5 to
59a46a8
Compare
62a2b91 to
4f68220
Compare
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.
Just a quick note on the embedder.h changes that I'll send out now while I continue to review.
|
I have removed the commit [Move FlutterBackingStorePresentInfo into FlutterBackingStore]. This moves |
4f68220 to
e22bfe6
Compare
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.
lgtm - thanks for your patience on this one, in particular on the embedder API changes.
ae5a33e to
d0c76bc
Compare
…133739) flutter/engine@1f1071d...1917fa9 2023-08-31 [email protected] Roll Skia from adaad6716b2c to 676a16152834 (1 revision) (flutter/engine#45314) 2023-08-31 [email protected] [macOS] Implement unobstructed platform views (flutter/engine#42960) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes flutter/flutter#129073
Changes to Embedder API
Introduced
FlutterRegionto represent arbitrary region:Note that this is identical to struct
FlutterDamagewith more generic naming. Maybe down the line we could deprecateFlutterDamageand useFlutterRegioninstead.Introduced
FlutterBackingStorePresentInfo:In future this struct may also contain more precise hit test region (when framework supports it) and/or information relevant to partial repaint (buffer damage, frame damage).
Added a
backing_store_present_infofield toFlutterLayer:Changes to the macOS embedder
This PR adds support for
FLTEnableSurfaceDebugInfoflag in main bundleInfo.plistthat enables visual indicators of overlay layers.Example of unobstructed platform views
surface-debug.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.