Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: flutter/engine
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: fe45a6608651
Choose a base ref
...
head repository: flutter/engine
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: fb6439918252
Choose a head ref
  • 12 commits
  • 39 files changed
  • 6 contributors

Commits on Nov 25, 2024

  1. removed unused variable for skia initialization (#56791)

    fixes flutter/flutter#159433
    
    test exempt: no functional change, removes unused code
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    gaaclarke authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    7426db2 View commit details
    Browse the repository at this point in the history
  2. [android] remove fml_check from surface_texture_external_texture (#56760

    )
    
    We may fail to acquire a new image from the external image source. When this happens, don't crash the app.
    
    Fixes flutter/flutter#159324
    Jonah Williams authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    f852b36 View commit details
    Browse the repository at this point in the history
  3. [iOS] Full keyboard access scrolling (#56606)

    This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 
    
    Previous PR for context: #55964
    
    https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac
    
    ### Why the UIScrollView subclass in the focus hierarchy
    
    The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.
    
    ### Things that currently may not work
    
    1. Nested scroll views (have not tried to verify) 
    
    The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).
    
    2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list
    
    Video demo (as you can see the scrolling is really finicky):
    
    https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978
    
    I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    LongCatIsLooong authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    7a8c1e8 View commit details
    Browse the repository at this point in the history
  4. Roll Dart SDK from df716eaa6ed2 to 4b49546a1dfa (1 revision) (#56793)

    https://dart.googlesource.com/sdk.git/+log/df716eaa6ed2..4b49546a1dfa
    
    2024-11-25 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.7.0-184.0.dev
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/dart-sdk-flutter-engine
    Please CC aaclarke@google.com,dart-vm-team@google.com on the revert to ensure that a human
    is aware of the problem.
    
    To file a bug in Flutter Engine: https://github.com/flutter/flutter/issues/new/choose
    
    To report a problem with the AutoRoller itself, please file a bug:
    https://issues.skia.org/issues/new?component=1389291&template=1850622
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    5db272c View commit details
    Browse the repository at this point in the history
  5. Roll Fuchsia Linux SDK from 9o0fWa2xVhmxV6Mtn... to 50xtjbMWWrqay_7m_…

    …... (#56795)
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/fuchsia-linux-sdk-flutter-engine
    Please CC aaclarke@google.com,zra@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    258666b View commit details
    Browse the repository at this point in the history
  6. [impeller] gles: started storing the number of handle deletions to av…

    …oid reallocation (#56799)
    
    I saw in profiles that we were spending a lot of time in
    push_back_slow_path. Reserving + emplacing removes that.
    test-except: no functional change, performance increase
    
    ## Pre-launch Checklist
    
    - [x] I read the [Contributor Guide] and followed the process outlined
    there for submitting PRs.
    - [x] I read the [Tree Hygiene] wiki page, which explains my
    responsibilities.
    - [x] I read and followed the [Flutter Style Guide] and the [C++,
    Objective-C, Java style guides].
    - [x] I listed at least one issue that this PR fixes in the description
    above.
    - [x] I added new tests to check the change I am making or feature I am
    adding, or the PR is [test-exempt]. See [testing the engine] for
    instructions on writing and running engine tests.
    - [x] I updated/added relevant documentation (doc comments with `///`).
    - [x] I signed the [CLA].
    - [x] All existing and new tests are passing.
    
    If you need help, consider asking for advice on the #hackers-new channel
    on [Discord].
    
    <!-- Links -->
    [Contributor Guide]:
    https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
    [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
    [test-exempt]:
    https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
    [Flutter Style Guide]:
    https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
    [C++, Objective-C, Java style guides]:
    https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    [testing the engine]:
    https://github.com/flutter/flutter/wiki/Testing-the-engine
    [CLA]: https://cla.developers.google.com/
    [flutter/tests]: https://github.com/flutter/tests
    [breaking change policy]:
    https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
    [Discord]: https://github.com/flutter/flutter/wiki/Chat
    gaaclarke authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    56d4ac0 View commit details
    Browse the repository at this point in the history
  7. iOS: Eliminate logging of non-zero origin platformviews (#56796)

    In #35501, handling was added to log a debug message to the console in the case where a platform view with a non-zero origin was identified.
    
    Unfortunately:
    * In unopt builds, the first thing we do in that block is to call FML_DCHECK asserting that the origin is zero, so we never actually emit the log statement.
    * In opt builds, FML_DCHECK is a no-op, so users are unlikely to actually ever notice the crash.
    
    The proper fix is to eliminate this restriction, but in the meantime, this eliminates this block entirely and leaves the TODO. We've had only two comments on that bug in the 2.5 years since it was added.
    
    Issue: flutter/flutter#109700
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    cbracken authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    3c751a9 View commit details
    Browse the repository at this point in the history
  8. [Impeller] better handle allocation herustics of Android slide in pag…

    …e transition. (#56762)
    
    For flutter/flutter#158881
    
    The new Android m3 page transition animates a saveLayer w/ opacity + slide in animation. See example video from linked PR:
    
    https://github.com/user-attachments/assets/1382374a-4e0c-4a0e-9d70-948ce12e6298
    
    On each frame, we intersect the coverage rect of this saveLayer contents with the screen rect, and shrink it to a partial rectangle. But this changes each frame, which forces us to re-allocate a large texture each frame, causing performance issues.
    
    Why not ignore the cull rect entirely? We sometimes ignore the cull rect for the image filter layer for similar reasons, but from observation, the sizes of these saveLayer can be slightly larger than the screen for flutter gallery. Instead, I attempted to use the cull rect size to shrink the saveLayer by shifting the origin before intersecting.
    
    I think this should be safe to do, as I believe it could only leave the coverage as larger than it would have been and not smaller.
    Jonah Williams authored Nov 25, 2024
    Configuration menu
    Copy the full SHA
    0d4d2cc View commit details
    Browse the repository at this point in the history

Commits on Nov 26, 2024

  1. Roll Skia from 12c8bd6ac1d9 to c1c8ff84997c (14 revisions) (#56801)

    https://skia.googlesource.com/skia.git/+log/12c8bd6ac1d9..c1c8ff84997c
    
    2024-11-25 danieldilan@google.com Revert "Change window::DisplayParams to be immutable and passed by pointer"
    2024-11-25 lukasza@chromium.org [rust png] Build / sources organization: Separate `decoder/` directory.
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from 73e40f43c062 to 64698c1a35b2 (1 revision)
    2024-11-25 kjlubick@google.com Change window::DisplayParams to be immutable and passed by pointer
    2024-11-25 danieldilan@google.com Use SkSafe32 functions when adding/subtracting deltas.
    2024-11-25 kjlubick@google.com Save 16 bytes on GrContextOptions allocations* by reordering fields
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skottie-base from e4021a6fc9aa to 52028e548417
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll debugger-app-base from 931df19ec335 to 9e05eb5b9edb
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll shaders-base from 99b73d05cdae to 7bdb025e3cbb
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll jsfiddle-base from 034839b9814b to 99d4627f212e
    2024-11-25 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skottie-base from c0ad379b6c58 to e4021a6fc9aa
    2024-11-25 kylechar@chromium.org Fix DefaultImageProvider::Make() leak
    2024-11-25 kjlubick@google.com Make SkGlyph and GrDriverBugWorkarounds trivially destructible
    2024-11-25 michaelludwig@google.com [graphite] Fix texture matrix for asyncReadPixelsYUV420
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/skia-flutter-autoroll
    Please CC aaclarke@google.com,brianosman@google.com,danieldilan@google.com on the revert to ensure that a human
    is aware of the problem.
    
    To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry
    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://issues.skia.org/issues/new?component=1389291&template=1850622
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Nov 26, 2024
    Configuration menu
    Copy the full SHA
    6ebfd9a View commit details
    Browse the repository at this point in the history
  2. Reverts "[iOS] Full keyboard access scrolling (#56606)" (#56802)

    Reverts: #56606
    Initiated by: LongCatIsLooong
    Reason for reverting: flutter/flutter#159456
    Original PR Author: LongCatIsLooong
    
    Reviewed By: {chunhtai, cbracken}
    
    This change reverts the following previous change:
    This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 
    
    Previous PR for context: #55964
    
    https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac
    
    ### Why the UIScrollView subclass in the focus hierarchy
    
    The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.
    
    ### Things that currently may not work
    
    1. Nested scroll views (have not tried to verify) 
    
    The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).
    
    2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list
    
    Video demo (as you can see the scrolling is really finicky):
    
    https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978
    
    I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    auto-submit[bot] authored Nov 26, 2024
    Configuration menu
    Copy the full SHA
    50174be View commit details
    Browse the repository at this point in the history
  3. Started caching HandleGLES's hash and made them immutable (#56800)

    flutter/flutter#159177
    test exempt: no functional change, performance
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    gaaclarke authored Nov 26, 2024
    Configuration menu
    Copy the full SHA
    8ad5709 View commit details
    Browse the repository at this point in the history
  4. iOS: Migrate PlatformViewsController to Objective-C (#56790)

    This migrates PlatformViewController from C++ to Objective-C. Generally, we try to keep the embedder interfaces and components written in Objective-C except for the few places where C++ interfaces are requried to interface with engine APIs such as Shell and PlatformView (e.g. the PlatformViewIOS subclass). Now that the implementation is Objective-C, the class and file are renamed to match Objective-C naming conventions.
    
    This allows us to take advantage of ARC and weak references, which eliminates the need for std::shared_ptr, fml::WeakPtr etc. Further, this eliminates some particularly unintuitive behaviour wherein this class was owned via a std::shared_ptr held by FlutterEngine, and injected into many other classes (e.g. AccessibilityBridge) via a std::shared_ptr& reference -- such that only one instance of the std::shared_ptr actually ever existed, presumably to avoid std::shared_ptr refcounting overhead. Given that this overhead was only incurred a single time at engine initialisation, this seems like overkill. One might ask why it wasn't therefore held in a `std::unique_ptr` and a `std::unique_ptr&` reference passed around. Likely, this was because we wanted to take a `fml::WeakPtr` reference on it.
    
    Regardless, none of this is necessary any longer now that we can inject `__weak FlutterPlatformViewsController*` instances to classes that use it.
    
    To be clear, this patch makes no attempt whatsoever to simplify or clean up the interface or implementation of this class. This class ties together far too many concepts and is injected into far too many places, and we should break it up and simplify it. However, the goal of this patch was simply to port to an Objective-C interface that plays nicely with the rest of the iOS embedder. This does include a couple minor cleanups in `#include`/`#import` order and usage to match our style guide.
    
    [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
    cbracken authored Nov 26, 2024
    Configuration menu
    Copy the full SHA
    fb64399 View commit details
    Browse the repository at this point in the history
Loading