[Windows] Update API for alternative Windows shell platform implementation #11327

clarkezone · 2019-08-21T00:25:27Z

#9835 introduced an alternative Windows shell platform implementation. This PR updates the API with a couple of changes:

The OS window is now created by the host app, not the engine
The host app asks the engine for a Flutter view and the host can then parent the resulting view into it's window hierarchy as it pleases using win32 parenting APIs
Since the host app now has direct control of the host window, removed a bunch of the wrapper methods for setting window properties as these can now be set directly on the window by the host.

In a future world where CoreWindow is supported as a native window type alongside HWNDs there would need to be another flavor of Flutter view.

There is a corresponding PR google/flutter-desktop-embedding#493 to switch the testbed and example projects over. That PR should not be accepted until the win32 implementation is the default for the engine.

There is a small amount of remaining cleanup needed hence this is WIP for now.

Part of flutter/flutter#38600

@stuartmorgan for FYI.

…urface on start

stuartmorgan-g

I wasn't able to spend a lot of time looking at the two patches together so I'm still missing some high-level understanding of the flow. Comments below from looking at this in isolation for now, and I'll look again this weekend if possible.

shell/platform/windows/client_wrapper/flutter_view_controller.cc

shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h

shell/platform/windows/client_wrapper/include/flutter/flutter_view.h

shell/platform/windows/client_wrapper/flutter_window_controller.cc

shell/platform/windows/flutter_windows.cc

shell/platform/windows/win32_flutter_window.h

canewsin · 2019-08-24T15:30:27Z

Since this commit is not merged Instead of Opening an Issue i was asking here that, will it possible to change attributes like title and icon of title bar, displayed or such basic things, with this implementation from attributes form flutter framework code
suppose if we implement a material app in flutter
we use title attribute will this can be parsed to displayed as window title bar on win32 window
and on a flutter we need a fullscreen app we use
SystemChrome.setEnabledUiOverlays([]);
will this possible on win32 implemetation
if not will all SystemChrome class variables are ignored ?

stuartmorgan-g · 2019-08-24T20:48:09Z

will it possible to change attributes like title and icon of title bar, displayed or such basic things, with this implementation from attributes form flutter framework code

There will no longer be framework methods to set all of those properties with this API, because it's no longer necessary; that was a limitation of GLFW. With this structure, setting those things would be done with normal Win32 APIs.

suppose if we implement a material app in flutter
we use title attribute will this can be parsed to displayed as window title bar on win32 window

That doesn't currently happen, so this PR won't change anything in that regard.

and on a flutter we need a fullscreen app we use
SystemChrome.setEnabledUiOverlays([]);
will this possible on win32 implemetation
if not will all SystemChrome class variables are ignored ?

SystemChrome's current methods are very mobile-centric, and probably don't make sense to implement on desktop as-is, but again that's unrelated to this specific change. Please feel free to file Flutter issues requesting specific desktop-related functionality you want to be available from the Dart side, as what/how those things are exposed in the framework is separate from the implementation in a given embedding, so doesn't need to wait for this to land.

canewsin · 2019-08-25T03:52:18Z

I will open issues related to dart side once this pr is merged and tested

clarkezone · 2019-08-25T21:44:12Z

All feedback bar one item addressed, merged in ToT. There is a failure in the bots that doesn’t look related.

canewsin · 2019-08-27T09:45:45Z

Will this Exposes Api to Draw Native Textures in Dart code ?

stuartmorgan-g · 2019-08-27T18:10:01Z

Will this Exposes Api to Draw Native Textures in Dart code ?

That is flutter/flutter#38601

This PR has a very specific scope. The place for general desktop or Windows feature requests is the Flutter issue tracker, not this PR; please limit comments here to things directly related to this PR.

stuartmorgan-g · 2019-08-27T18:12:10Z

There is a failure in the bots that doesn’t look related.

It is, actually; adding/renaming/deleting files requires updates to the golden file used by the license script. You need to apply the diff at the end of the failure output to ci/licenses_golden/licenses_flutter as part of this PR.

stuartmorgan-g

A couple of questions, but mostly just small nits. Looks great!

shell/platform/windows/win32_window.h

shell/platform/windows/win32_flutter_window.h

shell/platform/windows/win32_flutter_window.cc

shell/platform/windows/flutter_windows.cc

shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h

clarkezone · 2019-08-27T19:06:30Z

Ok will do

clarkezone · 2019-08-31T10:23:35Z

All feedback addressed, gates are now clean. I've also updated the corresponding PR in FDE to pick up the change to using HWND in the wrapper.

stuartmorgan-g

LGTM!

…mplementation (flutter/engine#11327)

…mplementation (flutter/engine#11327) (#39710) git@github.com:flutter/engine.git/compare/3deb8f5e45e1...51a376d git log 3deb8f5..51a376d --no-merges --oneline 2019-09-03 james@clarkezone.net [Windows] Update API for alternative Windows shell platform implementation (flutter/engine#11327) 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 garyq@google.com on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

clarkezone added 4 commits August 19, 2019 20:13

Begin API evolution to a more native win32 API

d4aced8

Child-window based hosting

81f0521

Plumb through an initial size for child window to avoid reallocated s…

f55e25f

…urface on start

Windows API cleanup part 1

aa40b34

googlebot added the cla: yes label Aug 21, 2019

clarkezone mentioned this pull request Aug 21, 2019

[Windows] Update example and testbed to use the native win32 Windows engine google/flutter-desktop-embedding#493

Merged

clarkezone added 5 commits August 21, 2019 11:17

Fix wrapper tests

97cd93a

Ensure flutter's HWND resources are destroyed

2321072

Final API cleanup

976fce5

Fix dynamic DPI handling

8d65758

Cleanup

a8e3c37

stuartmorgan-g self-requested a review August 22, 2019 02:58

Fix a bug that was causing engine to not be shutdown correctly

e131c6e

stuartmorgan-g reviewed Aug 23, 2019

View reviewed changes

clarkezone added 6 commits August 25, 2019 10:29

CR feedback

9548881

auto format

ea341c2

CR feedback: combine FlutterView and FlutterViewController

4c6a597

The one that clang-format seems to always get wrong

a7d8d3b

expletive

9425a4c

Merge remote-tracking branch 'upstream/master' into updateapiwin32

8e85523

stuartmorgan-g suggested changes Aug 27, 2019

View reviewed changes

fix sources for licesnse file

08f72bb

clarkezone added 5 commits August 31, 2019 01:55

CR Feedback

c5b422b

Merge remote-tracking branch 'upstream/master' into updateapiwin32

a64f45a

cleanup

4c074b5

Update GetNativeWindow() to return an HWND rather than a long

826fd44

fix formatting

2817553

stuartmorgan-g approved these changes Sep 2, 2019

View reviewed changes

clarkezone merged commit 51a376d into flutter:master Sep 3, 2019

clarkezone deleted the updateapiwin32 branch September 3, 2019 00:55

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 3, 2019

51a376d [Windows] Update API for alternative Windows shell platform i…

364b120

…mplementation (flutter/engine#11327)

engine-flutter-autoroll mentioned this pull request Sep 3, 2019

Roll engine 3deb8f5e45e1..51a376d73ba1 (1 commits) flutter/flutter#39710

Merged

[Windows] Update API for alternative Windows shell platform implementation #11327

[Windows] Update API for alternative Windows shell platform implementation #11327

Uh oh!

Conversation

clarkezone commented Aug 21, 2019 • edited by stuartmorgan-g Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

canewsin commented Aug 24, 2019

Uh oh!

stuartmorgan-g commented Aug 24, 2019

Uh oh!

canewsin commented Aug 25, 2019

Uh oh!

clarkezone commented Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canewsin commented Aug 27, 2019

Uh oh!

stuartmorgan-g commented Aug 27, 2019

Uh oh!

stuartmorgan-g commented Aug 27, 2019

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clarkezone commented Aug 27, 2019

Uh oh!

clarkezone commented Aug 31, 2019

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

clarkezone commented Aug 21, 2019 •

edited by stuartmorgan-g

Loading

clarkezone commented Aug 25, 2019 •

edited

Loading