engine icon indicating copy to clipboard operation
engine copied to clipboard

Published 20 hours ago • verified publisher icon flutter

Add platform view support for linux

Open huanghongxun opened this issue 3 years ago • 12 comments

This pull request is part of #24169. For framework API, see https://github.com/flutter/flutter/pull/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

  • [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 Hixie said 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.
  • [ ] 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.

huanghongxun avatar May 20 '21 14:05 huanghongxun

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

huanghongxun avatar May 20 '21 14:05 huanghongxun

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.

chinmaygarde avatar May 21 '21 18:05 chinmaygarde

when will it be merged?

wanjm avatar Jun 08 '21 00:06 wanjm

why does it blocked here

wanjm avatar Jul 10 '21 00:07 wanjm

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

huanghongxun avatar Aug 24 '21 12:08 huanghongxun

anything blocked?

wanjm avatar Nov 18 '21 23:11 wanjm

anything blocked?

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

huanghongxun avatar Nov 19 '21 18:11 huanghongxun

Maybe cc @cbracken and @gspencergoog ?

dkwingsmt avatar Jan 06 '22 08:01 dkwingsmt

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.

flutter-dashboard[bot] avatar Jan 26 '22 17:01 flutter-dashboard[bot]

@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 https://github.com/flutter/flutter/pull/74814) but I've got time to take a look.

cbracken avatar Mar 29 '22 23:03 cbracken

@cbracken rebased on main.

huanghongxun avatar Apr 02 '22 04:04 huanghongxun

Added some comments to https://github.com/flutter/flutter/pull/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 avatar May 24 '22 23:05 cbracken

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 avatar Aug 31 '22 00:08 cbracken