Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add platform view support for linux #26288

Closed
wants to merge 2 commits into from
Closed

Add platform view support for linux #26288

wants to merge 2 commits into from

Conversation

huanghongxun
Copy link
Contributor

This pull request is part of #24169. For framework API, see flutter/flutter#74814.

This pull request adds platform view API to Linux shell, without support for touch events and clipping mutations, which are splitted and proposed later when this PR get landed.

Code changes:

  1. FlPlatformView to wrap a GtkWidget, owned by FlPlatformViewsPlugin. This abstraction allows us to owning a FlMethodChannel in FlPlatformView for communication with dart codes.
  2. FlPlatformViewFactory for creating platform views for specific view type. Flutter Framework will call functions of FlPlatformViewFactory when creating a new platform view.
  3. FlPlatformViewsPlugin for implementing SystemChannels.platform_views from Flutter services library.

A working demo with webview: https://github.com/wechat-miniprogram/gtktest.

This pr does not support applying mutations on the platform views yet. I will open a PR later to implement that feature.

This pr is part of platform view implementation on Linux, related issue: flutter/flutter#41724.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.
  • The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla google-cla bot added the cla: yes label May 20, 2021
@huanghongxun
Copy link
Contributor Author

@robert-ancell @iskakaushik This PR contains parts other than FlGestureHelper and FlClippingView from #24169. Changes on FlView are simplified.

@huanghongxun huanghongxun mentioned this pull request May 20, 2021
Closed
9 tasks
@chinmaygarde
Copy link
Member

cc @gw280 as well. I know you are looking at the Mac side of things but it may be a good idea to go over this once to look for common patterns.

@wanjm
Copy link

wanjm commented Jun 8, 2021

when will it be merged?

This was referenced Jun 13, 2021
Closed
Open
@wanjm
Copy link

wanjm commented Jul 10, 2021

why does it blocked here

@huanghongxun
Copy link
Contributor Author

@robert-ancell Please take a look at this pr.

robert-ancell
robert-ancell reviewed Sep 3, 2021
View reviewed changes
shell/platform/linux/fl_platform_views_plugin_test.cc
const gchar* view_type,
FlValue* args,
GAsyncReadyCallback cb) {
FlValue* create_args = fl_value_new_map();
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing g_autoptr

Copy link
Contributor Author

@huanghongxun huanghongxun Sep 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

invoke_method takes ownership of create_args here. So we cannot free it after this method finished.

robert-ancell
robert-ancell approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@robert-ancell robert-ancell left a comment

Choose a reason for hiding this comment

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

Two small fixes required, but otherwise looks great. Thanks @huanghongxun!

robert-ancell
robert-ancell requested changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@robert-ancell robert-ancell left a comment

Choose a reason for hiding this comment

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

Changing status as a few small things to fix.

@CaseyHillers CaseyHillers changed the base branch from master to main November 15, 2021 18:16
@wanjm
Copy link

wanjm commented Nov 18, 2021

anything blocked?

@huanghongxun
Copy link
Contributor Author

anything blocked?

It seems that few people can review this and the dart side pull requests. And these people have no time.

@godofredoc godofredoc changed the base branch from master to main November 24, 2021 07:27
@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jan 6, 2022 •
edited
Loading

Maybe cc @cbracken and @gspencergoog ?

@gaaclarke gaaclarke changed the title Add platform view support Add platform view support for linux Jan 6, 2022
@goderbauer goderbauer mentioned this pull request Jan 12, 2022
Closed
8 tasks
@cbenhagen cbenhagen mentioned this pull request Jan 20, 2022
Open
@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

@maks maks mentioned this pull request Jan 31, 2022
Open
@dkwingsmt dkwingsmt removed their request for review March 15, 2022 22:16
@cbracken
Copy link
Member

@huanghongxun it's been a long while, but is this patch something you're willing to get rebased and tests fixed or would you like someone to pick this up and continue with it? Unfortunately we've been pretty short on people who could spend the appropriate amount of review time on this (and flutter/flutter#74814) but I've got time to take a look.

@cbracken cbracken self-requested a review March 29, 2022 23:44
@google-cla google-cla bot added cla: no and removed cla: yes labels Apr 1, 2022
@huanghongxun huanghongxun force-pushed the dev/platform_views_base branch from 5094123 to d906b26 Compare April 1, 2022 12:14
@google-cla google-cla bot added cla: yes and removed cla: no labels Apr 1, 2022
@huanghongxun
Copy link
Contributor Author

@cbracken rebased on main.

@cbracken
Copy link
Member

Added some comments to flutter/flutter#74814 covering the framework side of this work.

I think the embedder side is much less likely to be affected by the framework-side design work. I've pulled this patch down locally and am playing with it a bit to get a feel for it while I review. Thanks again for your patience on this.

@cbracken
Copy link
Member

cbracken commented Aug 31, 2022 •
edited
Loading

I'm going to close this for the time being, in order to get it off the review queue until we've worked out the right approach to consolidating the API in a way that works for all platforms. That's a problem being actively hacked out right now on macOS and Windows/Linux are on our agenda next. I do think that some of the consolidation work in the framework PR is something we want to followup on though.

Once we're set with macOS, we will absolutely want to get back to landing platform views for Linux.

@cbracken cbracken closed this Aug 31, 2022
@CaptainDario
Copy link

@cbracken Sorry for pinging you, but I have been following the platform view implementations already for quite some time.
As MacOS now landed (awesome work!) and the webview is also available in the latest webview package, I wanted to ask if now this PR could be looked at again.

@robert-ancell
Copy link
Contributor

I think this would have to be a new PR at this point as a lot of things have changed. There's also a lot of change occurring at the moment adding support for multi-view. However, getting platform views would be great too!

@wanjm
Copy link

wanjm commented Sep 23, 2024

@robert-ancell where can i get the progress of multi-window of linux;

@robert-ancell
Copy link
Contributor

@robert-ancell where can i get the progress of multi-window of linux;

See flutter/flutter#138178

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robert-ancell robert-ancell robert-ancell requested changes

@stuartmorgan stuartmorgan Awaiting requested review from stuartmorgan

@dnfield dnfield Awaiting requested review from dnfield

@gaaclarke gaaclarke Awaiting requested review from gaaclarke

@cbracken cbracken Awaiting requested review from cbracken

Assignees
No one assigned
Labels
cla: yes platform-linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@huanghongxun @chinmaygarde @wanjm @dkwingsmt @cbracken @CaptainDario @robert-ancell