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.

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

Add FlutterDesktopWindowProperties to the public API #133

Merged

Conversation

swift-kim
Copy link
Member

@swift-kim swift-kim commented Jun 30, 2021

  • Add FlutterProjectBundle and refactor similarly to the Windows embedder.
  • Allow relative paths as values of assets_path, icu_data_path, and aot_library_path.
  • Add FlutterDesktopWindowProperties to flutter_tizen.h. This might be used in the future.

Caveats:

  • If width or height in FlutterDesktopWindowProperties is set to non-zero, it is assumed that the screen is not resizable. Attempting to resize or rotate the screen is an undefined behavior.

This depends on #131 and should be rebased after #131 is merged.

@swift-kim
Copy link
Member Author

swift-kim commented Jun 30, 2021

The relative path thing doesn't work in case of the .NET app type because the executable path resolves to /usr/bin/dotnet-launcher but not the app's main assembly (bin/Runner.dll) path.

edit) It can't be applied to the native app case either because the executable path appears to be /usr/bin/launchpad-loader but not bin/runner in the same manner.

@swift-kim swift-kim requested a review from a team July 2, 2021 02:20
@swift-kim
Copy link
Member Author

I just removed the relative path support to avoid any confusion.

@swift-kim
Copy link
Member Author

I'll rebase and resolve any conflict when #125 is merged. You can still review the change.

@swift-kim swift-kim changed the title Refactor using FlutterProjectBundle Add FlutterDesktopWindowProperties to the public API Jul 6, 2021
@@ -233,20 +234,20 @@ uintptr_t TizenRendererEcoreWl2::GetWindowId() {

bool TizenRendererEcoreWl2::InitializeRenderer() {
int32_t width, height;
if (!SetupDisplay(width, height)) {
FT_LOGE("SetupDisplay fail");
if (!SetupDisplay(&width, &height)) {

Choose a reason for hiding this comment

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

if you use initial_geometry, maybe you don't really need this "width" and "height".

Copy link
Member Author

Choose a reason for hiding this comment

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

As you can see in the documentation of FlutterDesktopWindowProperties, if the width and height are not provided by the app explicitly, the values are obtained from the current display just as usual.

if (!engine->RunEngine(engine_properties)) {
flutter::FlutterProjectBundle project(engine_properties);
auto engine = std::make_unique<flutter::FlutterTizenEngine>(project);
if (window_properties.headed) {
Copy link
Member

Choose a reason for hiding this comment

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

I love this part! ❤️ It's much more intuitive. Thanks

Copy link
Member

@bwikbs bwikbs left a comment

Choose a reason for hiding this comment

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

Thanks It's pretty cool!

Copy link

@bbrto21 bbrto21 left a comment

Choose a reason for hiding this comment

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

LGTM!

shell/platform/tizen/flutter_tizen_engine.h Show resolved Hide resolved
return true;
}

void OnTerminate() {
printf("Shutting down the application...");
printf("Shutting down the application...\n");
Copy link

Choose a reason for hiding this comment

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

What do you think about modifying it as below and using the log macros?

--- a/shell/platform/tizen/tizen_log_stub.cc
+++ b/shell/platform/tizen/tizen_log_stub.cc
@@ -19,7 +19,7 @@ namespace flutter {
 void SetMinLoggingLevel(log_priority p){};
 
 log_priority GetMinLoggingLevel() {
-  return DLOG_ERROR;
+  return DLOG_DEBUG;
 };

Copy link
Member Author

@swift-kim swift-kim Jul 8, 2021

Choose a reason for hiding this comment

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

As we're already passing --verbose-logging as an engine argument, there's no need to change GetMinLoggingLevel.

I don't think it makes sense to use printf in OnCretae and FT_LOGI in OnTerminate. I think it's better to leave them as they are or remove both from OnCreate and OnTerminate.

Copy link

Choose a reason for hiding this comment

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

Yes, you can remove both and it's better than the current.

Copy link

Choose a reason for hiding this comment

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

yes, we can remove both and it is nice rather then as it is

@swift-kim swift-kim merged commit 39fd66f into flutter-tizen:flutter-2.2.1-tizen Jul 8, 2021
swift-kim added a commit that referenced this pull request Sep 27, 2021
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Nov 14, 2021
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Dec 9, 2021
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Dec 17, 2021
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Feb 7, 2022
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Feb 11, 2022
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request May 12, 2022
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Aug 5, 2022
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
swift-kim added a commit that referenced this pull request Sep 1, 2022
* Refactor using FlutterProjectBundle

* Unsupport relative paths

* Reland the support for relative paths

* Add FlutterDesktopWindowProperties to the public API

* Document undocumented public APIs

* Rebase and resolve conflicts

* Enable FlutterTizenEngineTest.Run_Twice

* Add documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants