core, wl, headless: New virtual functions in CogPlatform to track live viewport instances #683

aperezdc · 2023-12-18T11:01:18Z

Platform implementations may need (or want) to track all the CogViewport instances in use. Until now this was done by fetching the list of viewports from CogShell, but that assumed that a.) there is always a shell instantiated, and b.) there will ever be only one viewport. It is already possible today to use multiple viewports (see examples/viewports.c) so the latter does not hold true. We want to also avoid using the shell from platform implementations so what this PR does is:

Add two vfuncs CogPlatform.viewport_created and CogPlatform.viewport_disposed, and corresponding signals CogPlatform::viewport-created and CogPlatform::viewport-disposed. These are triggered from CogViewport during creation and disposal using private functions.
Add multiple view support to the headless platform plug-in, using the above to track all views in all viewports.
Change the Wayland platform plug-in to use the above to track viewports, instead of picking the viewport list from CogShell.

After this, the uses that remain of the shell in platform implementations are related to their configuration. Those uses will be addressed in a separa patch set.

This is part of the work towards decoupling CogShell from CogPlatform, as per #634.

Add a pair of new CogPlatform::viewport-{created,disposed} signals, available also as virtual functions. The main intended usage is tracking live CogViewport instances inside platform plug-in implementations.

Support more than one view. This is achieved by opting-in to use the new CogView.create_backend vfunc, which makes each web view responsible for creating its own WPE view backend. The platform keeps track of viewports using CogPlatform.viewport_{created,disposed}, and on each frame timer tick all views in from all viewports are checked for dispatching frame acknowledgements.

Use the new CogPlatform.viewport_{created,disposed} vfuncs to track live CogViewport instances, instead of relying on CogShell. This removes all the usage of the shell instance in the Wayland platform, paving the way to uncouple CogShell from CogPlatform.

psaavedra · 2023-12-19T08:54:06Z

platform/wayland/cog-platform-wl.c

-    wpe_fdo_initialize_for_egl_display(display->egl_display);
-
-    /* get a reference to the hashmap with viewports added to the shell */
-    self->viewports = cog_shell_get_viewports(shell);


The platform will use the new signals to track the Viewports by its own in the Platform::viewports. That is good.

What happens with the CogShell? it is still having its own list of Viewports accesible through cog_shell_get_viewports()?. Isn't redundant?

Yes, it is redundant to have cog_shell_get_viewports() and I plan to revamp CogShell in follow-up PRs. I focused in this one to only avoid using the shell to track viewports inside platform plug-ins.

Actually, scratch that, I will be adding one more commit here removing all the viewport handling from CogShell.

OK. good. Go ahead. Nice if we have all this done in the same PR.

I spent a bit of time removing from CogShell all the code related to view and viewport handling, and it's bigger than expected. For my current WIP patch I have:

5 files changed, 129 insertions(+), 413 deletions(-)

Now that is a nice amount of code removed, and there are opportunities to clean up things even more. Therefore I have changed my opinion again: let's merge this PR as-is, and leave the cleanups for follow-up PRs, to make them easier to review. WDYT @psaavedra?

Yes. My feeling was the change was not so big so it makes sense to do it in 2 steps.

platform/wayland/cog-platform-wl.c

psaavedra

R+

Remove handling of viewports and vies from CogShell. Tracking live viewports can be done using CogPlatform since #683 and platform plug-ins no longer depend on CogShell for that. The following are no longer needed in CogShell: - The :web-view and :viewport properties, and their accessors. - The ::create-view and ::create-viewport signals. - The .create_view() virtual function and its default implementation. - The cog_shell_add_viewport(), cog_shell_remove_viewport(), cog_shell_get_n_viewports(), and cog_shell_get_nth_viewport() methods. As a side effect of the above removals, the examples have been changed to have them create their views using cog_view_new() instead of relying on the default implementation of the CogShell.create_view() virtual function. The launcher also needed a number of changes, which now manages itself a viewport as it cannot rely on the shell for that. Some code was moved around to accommodate creation of the view in cog_launcher_startup() without relying on the CogShell::create-view signal, and similarly for web view creation when running in automation mode. There are more opportunities for further simplifications that would be better done in separate patches: this one focuses on the minimum amount of changes needed to get view and viewport handling out of CogShell.

Remove handling of viewports and views from CogShell. Tracking live viewports can be done using CogPlatform since #683 and platform plug-ins no longer depend on CogShell for that. The following are no longer needed in CogShell: - The :web-view and :viewport properties, and their accessors. - The ::create-view and ::create-viewport signals. - The .create_view() virtual function and its default implementation. - The cog_shell_add_viewport(), cog_shell_remove_viewport(), cog_shell_get_n_viewports(), and cog_shell_get_nth_viewport() methods. As a side effect of the above removals, the examples have been changed to have them create their views using cog_view_new() instead of relying on the default implementation of the CogShell.create_view() virtual function. The launcher also needed a number of changes, which now manages itself a viewport as it cannot rely on the shell for that. Some code was moved around to accommodate creation of the view in cog_launcher_startup() without relying on the CogShell::create-view signal, and similarly for web view creation when running in automation mode. There are more opportunities for further simplifications that would be better done in separate patches: this one focuses on the minimum amount of changes needed to get view and viewport handling out of CogShell.

Remove handling of viewports and views from CogShell. Tracking live viewports can be done using CogPlatform since Igalia#683 and platform plug-ins no longer depend on CogShell for that. The following are no longer needed in CogShell: - The :web-view and :viewport properties, and their accessors. - The ::create-view and ::create-viewport signals. - The .create_view() virtual function and its default implementation. - The cog_shell_add_viewport(), cog_shell_remove_viewport(), cog_shell_get_n_viewports(), and cog_shell_get_nth_viewport() methods. As a side effect of the above removals, the examples have been changed to have them create their views using cog_view_new() instead of relying on the default implementation of the CogShell.create_view() virtual function. The launcher also needed a number of changes, which now manages itself a viewport as it cannot rely on the shell for that. Some code was moved around to accommodate creation of the view in cog_launcher_startup() without relying on the CogShell::create-view signal, and similarly for web view creation when running in automation mode. There are more opportunities for further simplifications that would be better done in separate patches: this one focuses on the minimum amount of changes needed to get view and viewport handling out of CogShell.

aperezdc added the enhancement New feature or request label Dec 18, 2023

aperezdc requested a review from psaavedra December 18, 2023 11:01

aperezdc self-assigned this Dec 18, 2023

aperezdc requested review from carlosgcampos and alexgcastro December 18, 2023 11:01

aperezdc force-pushed the aperezdc/multiple-views branch from 6af7c01 to 908740f Compare December 18, 2023 11:29

aperezdc added 3 commits December 19, 2023 09:53

core: Add viewport tracking signals to CogPlatform

6df0e2d

Add a pair of new CogPlatform::viewport-{created,disposed} signals, available also as virtual functions. The main intended usage is tracking live CogViewport instances inside platform plug-in implementations.

aperezdc force-pushed the aperezdc/multiple-views branch from 908740f to 209676e Compare December 19, 2023 08:30

psaavedra reviewed Dec 19, 2023

View reviewed changes

aperezdc requested a review from psaavedra December 19, 2023 11:05

psaavedra approved these changes Dec 20, 2023

View reviewed changes

aperezdc merged commit 9e940ce into master Dec 20, 2023
5 checks passed

aperezdc deleted the aperezdc/multiple-views branch December 20, 2023 09:48

aperezdc mentioned this pull request Dec 20, 2023

core: Remove view and viewport handling from CogShell #685

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core, wl, headless: New virtual functions in CogPlatform to track live viewport instances #683

core, wl, headless: New virtual functions in CogPlatform to track live viewport instances #683

aperezdc commented Dec 18, 2023

psaavedra Dec 19, 2023

aperezdc Dec 19, 2023

aperezdc Dec 19, 2023

psaavedra Dec 19, 2023

aperezdc Dec 20, 2023

psaavedra Dec 20, 2023

psaavedra left a comment

core, wl, headless: New virtual functions in CogPlatform to track live viewport instances #683

core, wl, headless: New virtual functions in CogPlatform to track live viewport instances #683

Conversation

aperezdc commented Dec 18, 2023

psaavedra Dec 19, 2023

Choose a reason for hiding this comment

aperezdc Dec 19, 2023

Choose a reason for hiding this comment

aperezdc Dec 19, 2023

Choose a reason for hiding this comment

psaavedra Dec 19, 2023

Choose a reason for hiding this comment

aperezdc Dec 20, 2023

Choose a reason for hiding this comment

psaavedra Dec 20, 2023

Choose a reason for hiding this comment

psaavedra left a comment

Choose a reason for hiding this comment