Introduce CogViewport, add support in the Wayland plug-in and example program #644
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a new CogViewport class, which represents a container (e.g. a window, output surface, GTK widget, etc.) which can display one web view out from a group. The container can itself be observable using its "add" and "remove" signals, tracks a "visible" view, and ensures that only the visible view has the "visible" state flag enabled.
aperezdc
force-pushed
the
aperezdc/viewport
branch
from
6f5523f
to
574250d
Compare
|
One comment after a first quick test:
|
Thanks for testing, I hadn't tried the virtual keyboard support so I didn't notice. I will try to take a look later. |
psaavedra
reviewed
Nov 3, 2023
psaavedra
requested changes
Nov 3, 2023
There was a problem hiding this comment.
other than the mentioned regression found with the focus. The rest of the changes looks really great.
I also notice the view->platform is not really needed and removed from the code. I am fine with this also.
I think the PR were be ready for land once you update it according my suggestion and passes the code-style checker.
Use a CogViewport instead of a single web view. This will allow the current platform plug-ins to show in their output one of many views without needing many changes. Note that in the future platform plug-ins should be updated to avoid using globals and make them handle multiple viewports (e.g. windows) as well.
Add an examples/ subdirectory, and a new simple-stack example program which shows how to attach multiple views to the CogShell view stack and switch among them.
Replace the single view saved in the CogWlPlatform by the viewport being used by the shell, taking care that only the visible view gets to update the contents of the Wayland surface and that input events get sent to the visible view as well. There are some locations where the visible view is assumed to be also the focused one, which may not be true later on when multiple viewports are supported. This compromise is okay for the time being and may need to be revisited later on.
aperezdc
force-pushed
the
aperezdc/viewport
branch
from
574250d
to
5ebdb45
Compare
|
@psaavedra Updated, PTAL when you have a minute 🙏🏼 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings in the following:
CogViewportclass, which represents an “output” to display one of a set of view.CogShellto use a viewport instead of a single view. At some point we will want to uncouple the shell (see Disentangle CogShell from CogPlatform #634) but this replacement is enough for now.examples/viewport.cexample program which shows how to switch between among the views of a viewport.I would recommend reviewing each commit separately.