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

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Jan 5, 2021

Description

As shown in flutter.dev/go/multiple-engines, sharing graphics contexts between engines in a group opens up memory savings. This is one more optimization that is happening as part of the multiple-flutters work.

Related Issues

fixes flutter/flutter#72022

Tests

I added the following tests:

FlutterEngineTest unit tests.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@gaaclarke gaaclarke force-pushed the lfe-shared-gpu-context branch from ef5e5be to d0410e2 Compare January 5, 2021 23:02
@gaaclarke gaaclarke marked this pull request as ready for review January 5, 2021 23:25
@flutter-dashboard

This comment has been minimized.

@gaaclarke gaaclarke requested a review from xster January 5, 2021 23:25
@xster xster requested a review from chinmaygarde January 6, 2021 20:21
static_cast<flutter::PlatformViewIOS*>(platform_view.get());
std::shared_ptr<flutter::IOSContext> context = ios_platform_view->GetIosContext();
FML_DCHECK(context);
flutter::Shell::CreateCallback<flutter::PlatformView> on_create_platform_view =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether there might be a race here too and then noticed there was a comment in the other place where this callback was created to say the whole thing is synchronous. Worth copying that comment here too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- (void)tearDown {
}

- (void)testSpawnsShareGpuContext {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need mrc here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of our smart pointers around objective-c objects in headers. Certain internal headers for the engine can't be included without blowing up. In particular FlutterEngine_Internal.h. I tried to fix our smart pointers but it's a losing battle.

// |PlatformView|
void SetSemanticsEnabled(bool enabled) override;

const std::shared_ptr<IOSContext>& GetIosContext() { return ios_context_; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@gaaclarke gaaclarke merged commit ca156c5 into flutter:master Jan 7, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS: Share GPU contexts between lightweight engines

3 participants