Support top-layer mode #191

bbrto21 · 2021-10-18T11:59:03Z

Add top to FlutterDesktopWindowProperties as a member
If top is true, the window type will be NOTIFICATION with the top level priority

* Add top to FlutterDesktopWindowProperties as a member * If top is true, the window type will be NOTIFICATION with the top level priority Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 · 2021-10-18T12:01:04Z

Related PR :
flutter-tizen/tizen_tools/pull/19
flutter-tizen/flutter-tizen/pull/252

bbrto21 · 2021-10-18T12:09:12Z

You can create an app that always be on the top layer, as the below.

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

bbrto21 · 2021-10-19T01:44:04Z

@nojinjeong @HakkyuKim
Please let me know if this change meets your requirements.

swift-kim · 2021-10-19T01:58:54Z

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

+                            top_ ? ECORE_WL2_WINDOW_TYPE_NOTIFICATION
+                                 : ECORE_WL2_WINDOW_TYPE_TOPLEVEL);


Does EFL have any documentation for this enum? Do you know what the meaning of ECORE_WL2_WINDOW_TYPE_NOTIFICATION is? Shouldn't we use ECORE_WL2_WINDOW_TYPE_NONE instead of ECORE_WL2_WINDOW_TYPE_TOPLEVEL for normal windows?

I can't find any documents for this enum, but I found this

I guess the notification is what we are familiar with. The document says as follows.

A notification window, like a warning about battery life or a new E-Mail received.

ECORE_WL2_WINDOW_TYPE_NONE has the same meaning as ELM_WIN_UNKNOWN.

It's still not clear. What's different between ECORE_WL2_WINDOW_TYPE_NOTIFICATION and ECORE_WL2_WINDOW_TYPE_TOPLEVEL? The default value of top_level is true (flutter-tizen/flutter-tizen#252), so the window type of normal apps will default to ECORE_WL2_WINDOW_TYPE_NOTIFICATION after this change. Is this intentional?

The default value of top_level is true (flutter-tizen/flutter-tizen#252),

It's my mistake, I'll fix it.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

xuelian-bai · 2021-10-19T07:08:27Z

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

+      wl_display* displayWrapper =
+          static_cast<wl_display*>(wl_proxy_create_wrapper(wl_display_));
+      if (displayWrapper) {
+        wl_event_queue_ = wl_display_create_queue(wl_display_);


is it necessary to destroy this queue manually?

I added it, thank you for the review.

HakkyuKim · 2021-10-20T03:22:10Z

@nojinjeong @HakkyuKim Please let me know if this change meets your requirements.

@bbrto21
I tested on a mobile emulator and the feature itself is sufficient for the current requirements. I would also have to test it with the device in office this Friday. Thanks for the quick update!

nojinjeong · 2021-10-20T09:29:12Z

@nojinjeong @HakkyuKim Please let me know if this change meets your requirements.

I've talked with @HakkyuKim about the requirements, I would also have to test it with the device.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

shell/platform/tizen/BUILD.gn

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

WonyoungChoi · 2021-10-25T10:22:47Z

../../flutter/shell/platform/tizen/tizen_renderer_evas_gl.cc:13:10: fatal error: 'ui/efl_util.h' file not found

Oops. host build failed. I guess something should be added to

engine/ci/tizen/docker/Dockerfile

Line 21 in e010550

FROM build-engine-base AS build-engine-with-efl

swift-kim · 2021-10-25T10:42:01Z

~~@WonyoungChoi https://github.com/flutter-tizen/tizen_tools/commit/c5945f6c8c7b612cc91aec5eb5cddc4b68bcab24 has just been merged. Triggering the build again may fix the problem?~~

Ah, it looks like ui/efl_util.h is Tizen-only (not part of public EFL) and the related code should be excluded from the x64 build (__X64_SHELL__).

WonyoungChoi · 2021-10-25T11:02:16Z

@WonyoungChoi flutter-tizen/tizen_tools@c5945f6 has just been merged.

Triggering the build again may fix the problem?

sysroot will be used only for target builds. however, the workflow for pr build runs the host build to perform unit tests.

engine/.github/workflows/build.yml

Line 32 in e010550

- arch: x64

engine/ci/tizen/build-engine.sh

Line 46 in e010550

if [[ "$BUILD_OS" == "host" ]]; then

The host build case should be considered also.
https://github.com/flutter-tizen/flutter-tizen/wiki/Building-the-engine-from-source#building-the-linux-desktop-shell-experimental

* Fix naming convention Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 · 2021-10-26T01:55:24Z

Does anyone have any of the following problems when push?

/home/boram/Work/f-project/engine/src/out/host_debug/../../flutter/shell/platform/tizen/external_texture.h:18:10: error: 'GLES2/gl2.h' file not found [clang-diagnostic-error]
#include <GLES2/gl2.h>
         ^~~~~~~~~~~~~
Found compiler error(s).
:
1 error generated.
Error while processing /home/boram/Work/f-project/engine/src/flutter/shell/platform/tizen/channels/platform_view_channel.cc.
Found compiler error(s).

Lint problems found.
Starting formatting checks.
formatting checks finished in 0:00:04.562747
pre-push checks finished in 0:00:27.821834

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

swift-kim

Looks much cleaner!

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

shell/platform/tizen/tizen_renderer_ecore_wl2.h

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 · 2021-10-27T09:30:30Z

@swift-kim

Is it safe to busy wait like this?

I added repeat count, please review again :)

Can't tizen_policy_set_notification_level be called elsewhere?

I couldn't find any documentation related to this. so, I referred to the implementation of other modules in Tizen :(
do you know anything about this? If you can help me, I'd really appreciate it.

bwikbs

Thanks for your hard working!! It's really cool! ❤️

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

swift-kim · 2021-10-28T01:43:45Z

I'll publish a new engine release once this PR gets merged.

bbrto21 · 2021-11-01T06:33:51Z

As suggested by @xuelian-bai, there are a few things to check.
When I'm done checking, I'll remove no-merge label.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 · 2021-11-02T05:03:36Z

@xuelian-bai @swift-kim @bwikbs

Please, review again, I've updated this PR, it no longer uses global callbacks and busy waiting.
as suggested by @xuelian-bai over internal messenger, it retrieve global object to bind tizen policy.

swift-kim

Some nitpicks :)

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

shell/platform/tizen/tizen_renderer_ecore_wl2.h

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bwikbs · 2021-11-02T06:01:26Z

For a while, I've been looking for wl_display_dispatch_queue_pending 😄
I didn't have a complaint at busy waiting, but now it's much more clean & nit! 👍

xuelian-bai · 2021-11-02T06:07:03Z

shell/platform/tizen/tizen_renderer_ecore_wl2.cc

+            wl_registry_bind(registry, global->id, &tizen_policy_interface, 1));
+        break;
+      }
    }


'itr' may be destroyed by eina_iterator_free

I found the below note. Thank you.

Note The caller of this function should free the returned Eina_Iterator when finished with it.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

xuelian-bai

LGTM

* Support top-layer mode * Add top to FlutterDesktopWindowProperties as a member * If top is true, the window type will be NOTIFICATION with the top level priority Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Update based on review Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Destroy wl_event_queue Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Fix build break on host * Fix naming convention Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Remove unnecessary code Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Add repeat count to prevent infinite wait Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Retrieve global objects to bind tizen policy Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Update based on review Signed-off-by: Boram Bae <boram21.bae@samsung.com> * Free Eina_Iterator after using it Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Support top-layer mode * Add top to FlutterDesktopWindowProperties as a member * If top is true, the window type will be NOTIFICATION with the top level priority * Destroy wl_event_queue * Fix build break on host * Add repeat count to prevent infinite wait * Retrieve global objects to bind tizen policy * Update based on review * Free Eina_Iterator after using it Signed-off-by: Boram Bae <boram21.bae@samsung.com>

Support top-layer mode

5e3f387

* Add top to FlutterDesktopWindowProperties as a member * If top is true, the window type will be NOTIFICATION with the top level priority Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bwikbs reviewed Oct 18, 2021

View reviewed changes

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

bbrto21 requested review from HakkyuKim and nojinjeong October 19, 2021 01:22

swift-kim reviewed Oct 19, 2021

View reviewed changes

Update based on review

1fcc78f

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

xuelian-bai reviewed Oct 19, 2021

View reviewed changes

Destroy wl_event_queue

ab500d7

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 marked this pull request as ready for review October 25, 2021 08:22

swift-kim reviewed Oct 25, 2021

View reviewed changes

shell/platform/tizen/BUILD.gn Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

Fix build break on host

d3d5b1a

* Fix naming convention Signed-off-by: Boram Bae <boram21.bae@samsung.com>

swift-kim reviewed Oct 26, 2021

View reviewed changes

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

Remove unnecessary code

c1f72fb

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

swift-kim reviewed Oct 26, 2021

View reviewed changes

Add repeat count to prevent infinite wait

bd246af

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bwikbs approved these changes Oct 27, 2021

View reviewed changes

swift-kim reviewed Oct 28, 2021

View reviewed changes

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

HakkyuKim mentioned this pull request Oct 28, 2021

Add window channel #201

Merged

7 tasks

swift-kim approved these changes Oct 29, 2021

View reviewed changes

bbrto21 added the no merge This PR is not ready for merge yet label Nov 1, 2021

Retrieve global objects to bind tizen policy

1360e8b

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

bbrto21 removed the no merge This PR is not ready for merge yet label Nov 2, 2021

swift-kim reviewed Nov 2, 2021

View reviewed changes

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.cc Outdated Show resolved Hide resolved

shell/platform/tizen/tizen_renderer_ecore_wl2.h Show resolved Hide resolved

Update based on review

bd6df39

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

xuelian-bai reviewed Nov 2, 2021

View reviewed changes

Free Eina_Iterator after using it

a6ca740

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

xuelian-bai approved these changes Nov 2, 2021

View reviewed changes

swift-kim merged commit ad8d61f into flutter-tizen:flutter-2.5.1-tizen Nov 2, 2021

		top_ ? ECORE_WL2_WINDOW_TYPE_NOTIFICATION
		: ECORE_WL2_WINDOW_TYPE_TOPLEVEL);

Support top-layer mode #191

Support top-layer mode #191

Uh oh!

Conversation

bbrto21 commented Oct 18, 2021

Uh oh!

bbrto21 commented Oct 18, 2021

Uh oh!

bbrto21 commented Oct 18, 2021

Uh oh!

Uh oh!

bbrto21 commented Oct 19, 2021

Uh oh!

swift-kim Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

swift-kim Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

xuelian-bai Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HakkyuKim commented Oct 20, 2021

Uh oh!

nojinjeong commented Oct 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WonyoungChoi commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swift-kim commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WonyoungChoi commented Oct 25, 2021

Uh oh!

bbrto21 commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

swift-kim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbrto21 commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwikbs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swift-kim commented Oct 28, 2021

Uh oh!

bbrto21 commented Nov 1, 2021

Uh oh!

bbrto21 commented Nov 2, 2021

Uh oh!

swift-kim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbrto21 Oct 25, 2021 •

edited

Loading

WonyoungChoi commented Oct 25, 2021 •

edited

Loading

swift-kim commented Oct 25, 2021 •

edited

Loading

bbrto21 commented Oct 26, 2021 •

edited

Loading

bbrto21 commented Oct 27, 2021 •

edited

Loading