Separate builds for profiles by swift-kim · Pull Request #67 · flutter-tizen/engine

swift-kim · 2021-04-19T03:44:02Z

Re-write the build script for Tizen (tizen/BUILD.gn)
- Learn GN first if you want to fully understand the script - it's not that difficult!
- Generates 4 outputs:
  - libflutter_tizen_mobile.so (Ecore Wl2)
  - libflutter_tizen_wearable.so (Evas GL)
  - libflutter_tizen_tv.so (Ecore Wl2)
  - libflutter_tizen_common.so (Ecore Wl2)
Use __dlog_print() only on TV and use dlog_print() on other profiles
Cherry-pick "Enable Evas GL direct mode (Enable Evas GL direct mode #54)" (+ build fix)
Unsupport the ecore_wl renderer (no more separate build for Tizen 4.0 because Evas GL supports Tizen 4.0)
Update azure-pipelines.yml accordingly

This change requires https://github.com/flutter-tizen/tizen_tools/pull/9.

Contributes to flutter-tizen/flutter-tizen#86 and flutter-tizen/flutter-tizen#21.

More testing, improvements, and clean-ups are needed (I tested only on a few devices).

shell/common/animator.cc

shell/platform/tizen/channels/text_input_channel.cc

shell/platform/tizen/tizen_renderer_evas_gl.cc

shell/platform/tizen/tizen_renderer.cc

swift-kim · 2021-04-22T05:36:27Z

I tested on

Tizen 4.0 wearable emulator
Tizen 5.5 wearable emulator
Tizen 6.0 mobile emulator
Tizen 6.0 TV emulator
Tizen 5.5 watch
Tizen 6.0 watch
Tizen 6.0 rpi4 (arm64)

jsonexample and gallery worked fine on every device. (I didn't measure performance but there was no obvious regression.) Currently the video_player plugin doesn't work properly in Evas GL mode (watch) - this will be addressed later.

bbrto21

Generally, LGTM
However, I would like other reviewers to review Evas GL related things (loop, render and etc), because there were no reviewers other than those who participated in the PoC at that time

shell/platform/tizen/channels/text_input_channel.cc

shell/platform/tizen/tizen_renderer_evas_gl.h

shell/platform/tizen/tizen_event_loop.h

shell/platform/tizen/tizen_renderer_evas_gl.cc

- Cherry-pick the commit from flutter-2.0.1-tizen-dev - Rename FLUTTER_TIZEN_EVASGL with TIZEN_RENDERER_EVAS_GL - Add missing switches and fix build errors Co-authored-by: Boram Bae <boram21.bae@samsung.com> Co-authored-by: MuHong Byun <mh.byun@samsung.com>

…tr_t

swift-kim · 2021-04-23T01:52:17Z

Resolved a conflict with #69.

rwalczyna · 2021-04-23T12:44:27Z

shell/platform/tizen/tizen_event_loop.cc

+  for (const auto& task : expired_tasks_) {
+    on_task_expired_(&task);
+  }
+  expired_tasks_.clear();


Shouldn't we lock expired_tasks_mutex_ here?

I designed to lock the mutex in the caller of OnTaskExpired if necessary.

@bbrto21 Apparently the caller (ExcuteTaskEvents) of OnTaskExpired doesn't hold the lock and the TizenRenderEventLoop::OnTaskExpired() implementation uses the lock. I'm not still sure what you intended.

@swift-kim Well, now that I've reviewed it again.... I think that it looks good to lock the mutex here too. It's been so long since I implemented this part, so I can't remember exactly what I was thinking then. 😁 😁 😁

bbrto21

LGTM

pwasowski2 · 2021-04-28T07:59:05Z

shell/platform/tizen/BUILD.gn

+    use_evas_gl_renderer = false
  }
+
+  shared_library("flutter_tizen_${target_name}") {


I've found something confusing (at least for gn novice): the value of target_name within the body of shared_library, i.e. between lines 85 and 164, is changed from mobile/werable/tv/common to flutter_tizen_mobile/flutter_tizen_werable/flutter_tizen_tv/flutter_tizen_common respectively.

I'm not sure, if it's an issue, but it causes "undefined reference" errors when using code similar to the one from the diff from this comment #65 (comment) to extend libs, i.e.:

+ if (target_name == "mobile") { # should be target_name == "flutter_tizen_mobile" + libs += [ "cbhm" ] + }

I've printed the target_name values before, within and after shared_library() call (using this code: pwasowski2@0a41130) and here's what gn uses as target_names:

target_name before 'shared_library()': mobile target_name in 'shared_library()': flutter_tizen_mobile target_name == "mobile" is FALSE target_name == "flutter_tizen_mobile" is TRUE target_name after 'shared_library()': mobile ...

@swift-kim, is this change of target_name value desired?

@pwasowski2 You're right. My comment was misleading because I didn't test the change suggested in the comment.

You can either:

Use invoker.target_name (instead of target_name) inside shared_library()

Define a new variable (e.g. profile = target_name) outside shared_library() and use that inside shared_library()

Actually there is a caveat about this in the GN documentation: https://gn.googlesource.com/gn/+/master/docs/reference.md#var_target_name

Thanks for the explanation :)

* Separate binaries * Clean up the build script * Use templates * Use __dlog_print() only on TV * Copy headers and icudata * Partially update azure-pipelines.yml * Temporarily unsupport Tizen 4.0 and use only ecore_wl2 * Cherry-pick "Enable Evas GL direct mode (#54)" - Cherry-pick the commit from flutter-2.0.1-tizen-dev - Rename FLUTTER_TIZEN_EVASGL with TIZEN_RENDERER_EVAS_GL - Add missing switches and fix build errors Co-authored-by: Boram Bae <boram21.bae@samsung.com> Co-authored-by: MuHong Byun <mh.byun@samsung.com> * Fix CI build * Disable Evas GL direct mode and remove use of elm_win_aux_hint_add * Simplify the CI job * A workaround for isIME in Evas GL mode * Refactor: Clean up tizen_renderer.h and re-order functions * Refactor: Rename GetEcoreWindowId and change its return type to uintptr_t * Refactor: Make GetImageHandle() Evas GL-only * Refactor: Additional clean ups * Initialize members properly * Get window id from evas_window_ * Re-format code Co-authored-by: Boram Bae <boram21.bae@samsung.com> Co-authored-by: MuHong Byun <mh.byun@samsung.com>

* Separate binaries * Clean up the build script * Use templates * Use __dlog_print() only on TV * Copy headers and icudata * Partially update azure-pipelines.yml * Temporarily unsupport Tizen 4.0 and use only ecore_wl2 * Cherry-pick "Enable Evas GL direct mode (#54)" * Fix CI build * Disable Evas GL direct mode and remove use of elm_win_aux_hint_add * Simplify the CI job * A workaround for isIME in Evas GL mode * Refactor: Clean up tizen_renderer.h and re-order functions * Refactor: Rename GetEcoreWindowId and change its return type to uintptr_t * Refactor: Make GetImageHandle() Evas GL-only * Refactor: Additional clean ups * Initialize members properly * Get window id from evas_window_ * Re-format code Co-authored-by: Boram Bae <boram21.bae@samsung.com> Co-authored-by: MuHong Byun <mh.byun@samsung.com>

bbrto21 reviewed Apr 19, 2021

View reviewed changes

shell/common/animator.cc Outdated Show resolved Hide resolved

shell/platform/tizen/channels/text_input_channel.cc Show resolved Hide resolved

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

bbrto21 reviewed Apr 19, 2021

View reviewed changes

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

swift-kim force-pushed the embedders-for-profiles branch from 9ca99b8 to 6115693 Compare April 19, 2021 07:54

swift-kim marked this pull request as ready for review April 22, 2021 03:39

swift-kim requested a review from a team April 22, 2021 05:28

bbrto21 reviewed Apr 22, 2021

View reviewed changes

This was referenced Apr 22, 2021

Use different embedders for different profiles flutter-tizen/flutter-tizen#91

Merged

Some unresolved issues #53

Closed

swift-kim and others added 17 commits April 23, 2021 10:47

Separate binaries

42b9041

Clean up the build script

b8251fa

Use templates

19b8bb8

Use __dlog_print() only on TV

0de970f

Copy headers and icudata

00bf7ac

Partially update azure-pipelines.yml

30223bc

Temporarily unsupport Tizen 4.0 and use only ecore_wl2

4a4181a

Fix CI build

217caf3

Disable Evas GL direct mode and remove use of elm_win_aux_hint_add

6d44714

Simplify the CI job

6cfaab9

A workaround for isIME in Evas GL mode

26f4fa5

Refactor: Clean up tizen_renderer.h and re-order functions

d2ec02f

Refactor: Rename GetEcoreWindowId and change its return type to uintp…

2210172

…tr_t

Refactor: Make GetImageHandle() Evas GL-only

66fcc5f

Refactor: Additional clean ups

ba4c936

Initialize members properly

97ef634

swift-kim force-pushed the embedders-for-profiles branch from e8f434f to 97ef634 Compare April 23, 2021 01:51

swift-kim mentioned this pull request Apr 23, 2021

Current limitations of the Evas GL renderer flutter-tizen/embedder#7

Open

7 tasks

rwalczyna reviewed Apr 23, 2021

View reviewed changes

swift-kim mentioned this pull request Apr 26, 2021

Remove FlutterWindowProperties and get the screen size automatically #72

Merged

swift-kim added 2 commits April 26, 2021 15:31

Get window id from evas_window_

f4c250f

Re-format code

0113bff

swift-kim requested a review from a team April 26, 2021 08:16

This was referenced Apr 26, 2021

Remove the use of deprecated FlutterWindowProperties flutter-tizen/flutter-tizen#93

Merged

Publish: Undocumented APIs is detected (allow list) flutter-tizen/flutter-tizen#21

Closed

bbrto21 approved these changes Apr 27, 2021

View reviewed changes

bbrto21 merged commit 52704ae into flutter-tizen:flutter-2.0.1-tizen Apr 27, 2021

swift-kim mentioned this pull request Apr 27, 2021

[Clipboard] getData implementation added #65

Closed

pwasowski2 reviewed Apr 28, 2021

View reviewed changes

swift-kim mentioned this pull request May 3, 2021

Temporary fix for the invalid tbm_surface error #80

Closed

swift-kim pushed a commit that referenced this pull request Sep 1, 2022

Chainable texture filters (#67)

b3cbf05

Conversation

swift-kim commented Apr 19, 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!

Uh oh!

swift-kim commented Apr 22, 2021

Uh oh!

bbrto21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

swift-kim commented Apr 23, 2021

Uh oh!

rwalczyna Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

swift-kim Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

bbrto21 left a comment

Choose a reason for hiding this comment

Uh oh!

pwasowski2 Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

swift-kim Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

pwasowski2 Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

swift-kim commented Apr 19, 2021 •

edited

Loading