Skip to content

Refactor renderer to support vulkan extensions #105

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Jul 18, 2025
Merged

Refactor renderer to support vulkan extensions #105

merged 15 commits into from
Jul 18, 2025

Conversation

@xiaowei-guan
Copy link
Contributor

@xiaowei-guan xiaowei-guan commented Jun 27, 2025 •
edited
Loading

#101
Main changes:

  • Make FlutterTizenView SetEngine and CreateRenderSurfcae methods private.
  • Add TizenRendererNui class to decorate GL renderer for NUI.
  • Remove GL renderer methods from FlutterTizenView.
  • Do not call CreateSurface and DestorySurface in FlutterTizenView.
  • Move GetRendererConfig to renderer.
  • Extract a new class for GL renderer.
  • Move create texture code from texture registrar to renderer.

@xiaowei-guan
Refactor FlutterTizenView
8b03d53 
Make SetEngine and CreateRenderSurfcae private.
@xiaowei-guan
Remove tizen renderer methods from FlutterTizenView
fd412b3 
1. Add TizenRendererNui class to decorate GL renderer.
@xiaowei-guan
Do not call CreateSurface and DestorySurface in FlutterTizenView
21d363d 
1.Call CreateSurface when new renderer.
2.Call DestorySurface when desconstruct renderer.
@xiaowei-guan
Move GetRendererConfig to renderer
6025c58
@xiaowei-guan
Add TizenRendererGL
445a7c9
@xiaowei-guan
Fix build error and code format issue
c5aef70
@xiaowei-guan xiaowei-guan marked this pull request as draft June 27, 2025 08:59
@xiaowei-guan xiaowei-guan force-pushed the refactor-renderer-001 branch from 863bb70 to 908e900 Compare June 30, 2025 10:28
@xiaowei-guan
Move create texture code from texture registrar to renderer
908e900
@xiaowei-guan
Fix texture registrar crash issue
8121f5e 
1.Fix texture registrar crash issue.
2.Temporary remove texture registrar unit test code, we need
re-implement this unit test code.
@xiaowei-guan
Fix NUI view not show issue
f01b482
@JSUYA
Copy link
Member

JSUYA commented Jul 3, 2025

Hi about this commit 8121f5e ...
There was a similar case at #90.
Did you check window's code?

As the feature of tizen embedder has modified, there have been cases where we needed additional mock-up of the embedder api.
I haven't tested it yet, but if you look at flutter_windows_texture_registrar_unittests.cc in windows, you might need to add a mock-up of PostRenderThreadTask.
(flutter/engine/src/flutter/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc)

@xiaowei-guan xiaowei-guan marked this pull request as ready for review July 4, 2025 01:42
@xiaowei-guan
Copy link
Contributor Author

xiaowei-guan commented Jul 4, 2025

Hi about this commit 8121f5e ... There was a similar case at #90. Did you check window's code?

As the feature of tizen embedder has modified, there have been cases where we needed additional mock-up of the embedder api. I haven't tested it yet, but if you look at flutter_windows_texture_registrar_unittests.cc in windows, you might need to add a mock-up of PostRenderThreadTask. (flutter/engine/src/flutter/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc)

Fail to run texture unittest, because texture registrar fail to create ExternalTexture. Not related to Embedder API.
I added a macro definition for this unit tests.

JSUYA
JSUYA reviewed Jul 7, 2025
View reviewed changes
JSUYA
JSUYA reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Have you tried testing ElmFlutterView? I tested NUIFlutterView, but videos and webviews don't seem to work as expected (I think this was the case even before this PR). It can be a problem that my TV is running on Tizen 10.0 dev.

seungsoo47
seungsoo47 reviewed Jul 7, 2025
View reviewed changes
@xiaowei-guan xiaowei-guan force-pushed the refactor-renderer-001 branch from 0be7233 to c5b066d Compare July 8, 2025 06:54
@xiaowei-guan
Fix code review issues
c5b066d 
1.Re-implement createRenderer method
2.Remove empty line
2.Supplement header file path
@xiaowei-guan
Revert GetFlutterTransformation to flutter tizen view
fb6350d
JSUYA
JSUYA reviewed Jul 8, 2025
View reviewed changes
@xiaowei-guan
Make CreateSurface and DestorySurface protected
6634d18 
CreateSurface method only called by TizenRendereer and it's child class.
@xiaowei-guan
Merge remote-tracking branch 'upstream/master' into refactor-renderer…
824f59e 
…-001
@xiaowei-guan
Copy link
Contributor Author

xiaowei-guan commented Jul 10, 2025

Have you tried testing ElmFlutterView? I tested NUIFlutterView, but videos and webviews don't seem to work as expected (I think this was the case even before this PR). It can be a problem that my TV is running on Tizen 10.0 dev.

I have testted the ElmFlutterView on Emulator,RPI4 and TV devices. Only video_play plugin not show issue on Tizen 10.0 device, and old version has the same issue, I think this is a platfrom issue.

I also tested the NUIFlutterView, only video_play plugin has video frozen issue on Tizen 10.0, the old version has the same issue.

@JSUYA
Copy link
Member

JSUYA commented Jul 18, 2025

@xiaowei-guan If there are no more problems, could you merge this?

@xiaowei-guan
Copy link
Contributor Author

xiaowei-guan commented Jul 18, 2025

@xiaowei-guan If there are no more problems, could you merge this?

Ok

@xiaowei-guan xiaowei-guan merged commit 361fc56 into flutter-tizen:master Jul 18, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JSUYA JSUYA JSUYA approved these changes

@seungsoo47 seungsoo47 seungsoo47 approved these changes

@gin7773 gin7773 Awaiting requested review from gin7773

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xiaowei-guan @JSUYA @seungsoo47