[PATCH weston 08/12 v2] weston: Port Wayland backend to new output handling API
Pekka Paalanen
ppaalanen at gmail.com
Thu Aug 18 10:31:22 UTC 2016
On Wed, 17 Aug 2016 19:49:06 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:
> On 17.08.2016 17:07, Pekka Paalanen wrote:
> > On Sun, 14 Aug 2016 17:28:17 +0200
> > Armin Krezović <krezovic.armin at gmail.com> wrote:
> >
> >> This is a complete port of the Wayland backend that
> >> uses recently added output handling API for output
> >> configuration.
> >>
> >> - Output can be configured at runtime by passing the
> >> necessary configuration parameters, which can be
> >> filled in manually, obtained from the configuration
> >> file or obtained from the command line using
> >> previously added functionality. It is required that
> >> the scale and transform values are set using the
> >> previously added functionality.
> >>
> >> - Output can be created at runtime using the output
> >> API. The output creation only creates a pending
> >> output, which needs to be configured the same way as
> >> mentioned above.
> >>
> >> However, the backend can behave both as windowed backend
> >> and as a backend that issues "hotplug" events, when
> >> running under fullscreen shell or with --sprawl command
> >> line option. The first case was covered by reusing
> >> previously added functionality. The second case required
> >> another API to be introduced and implemented into both
> >> the backend and compositor for handling output setup.
> >>
> >> After everything has been set, output needs to be
> >> enabled manually using weston_output_enable().
> >>
> >> v2:
> >>
> >> - Fix wet_configure_windowed_output_from_config() usage.
> >> - Call wayland_output_disable() explicitly from
> >> wayland_output_destroy().
> >>
> >> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> >> ---
> >> compositor/main.c | 275 +++++++++++++++-------------------
> >> libweston/compositor-wayland.c | 327 +++++++++++++++++++++++------------------
> >> libweston/compositor-wayland.h | 27 +++-
> >> 3 files changed, 325 insertions(+), 304 deletions(-)
> >>
> >> diff --git a/compositor/main.c b/compositor/main.c
> >> index a72c2f9..143ee21 100644
> >> --- a/compositor/main.c
> >> +++ b/compositor/main.c
> >> -/*
> >> - * Append a new output struct at the end of new_config.outputs and return a
> >> - * pointer to the newly allocated structure or NULL if fail. The allocated
> >> - * structure is NOT cleared nor set to default values.
> >> - */
> >> -static struct weston_wayland_backend_output_config *
> >> -weston_wayland_backend_config_add_new_output(struct weston_wayland_backend_config *config)
> >> +wayland_backend_output_configure(struct wl_listener *listener, void *data)
> >> {
> >> - struct weston_wayland_backend_output_config *outputs;
> >> - const size_t element_size = sizeof(struct weston_wayland_backend_output_config);
> >> + struct weston_output *output = data;
> >> + struct wet_output_config defaults = {
> >> + .width = 1024,
> >> + .height = 640,
> >> + .scale = 1,
> >> + .transform = WL_OUTPUT_TRANSFORM_NORMAL
> >> + };
> >>
> >> - outputs = realloc(config->outputs,
> >> - (config->num_outputs + 1) * element_size);
> >> - if (!outputs)
> >> - return NULL;
> >> - config->num_outputs += 1;
> >> - config->outputs = outputs;
> >> - return &(config->outputs[config->num_outputs - 1]);
> >> + if (wet_configure_windowed_output_from_config(output, &defaults) < 0)
> >> + weston_log("Cannot configure output \"%s\".\n",
> >> + output->name ? output->name : "unnamed");
> >
> > I think wayland_backend_output_configure() is missing a special case
> > for --fullscreen. That is, desktop shell (windowed api) but asked to be
> > fullscreen...
> >
> > Actually no, this is fine. It would just need a comment explaining that
> > the backend will override the size if --fullscreen was given. I see
> > this would be inconvenient to realize otherwise.
> >
> >
>
> The backend would only set the surface fullscreen. I don't know what it will
> do to the size though, the values aren't touched, at least not that I saw it.
Setting fullscreen involves a shell protocol hand-shake to negotiate
the size. The backend should be doing that already.
> >> }
> >>
> >> static int
> >> -load_wayland_backend_config(struct weston_compositor *compositor, int *argc,
> >> - char *argv[], struct weston_config *wc,
> >> - struct weston_wayland_backend_config *config)
> >> +load_wayland_backend(struct weston_compositor *c,
> >> + int *argc, char **argv, struct weston_config *wc)
> >
> > The new load_wayland_backend() looks quite clean, in spite of some
> > mixed code and declarations issue, compared to the old
> > load_wayland_backend_config(). That's nice.
> >
> >> {
> >> + struct weston_wayland_backend_config config = {{ 0, }};
> >> @@ -1038,118 +1043,169 @@ wayland_output_create(struct wayland_backend *b, int x, int y,
> >> output->parent.surface);
> >> if (!output->parent.shell_surface)
> >> goto err_surface;
> >> +
> >> wl_shell_surface_add_listener(output->parent.shell_surface,
> >> &shell_surface_listener, output);
> >> }
> >>
> >> - if (fullscreen && b->parent.shell) {
> >> + if (!b->sprawl_across_outputs && b->fullscreen && b->parent.shell) {
> >
> > Hmm, fullscreen was limited to exactly one output, so sprawl with it
> > does not make sense... I suppose this hunk is ok, but still looks a
> > little out of place in this patch. What prompted
> > adding !b->sprawl_across_outputs && ?
> >
>
> sprawl_across_outputs is set for both fs_shell and --sprawl, which is handled
> down below and a bit different. This is supposed to be a safeguard, since one
> can pass both --sprawl and --fullscreen, and all that while on desktop shell.
I wonder if --sprawl --fullscreen is even supposed to work. Ok.
> >> +static int
> >> +wayland_output_configure_hotplug(struct weston_output *base)
> >> +{
> >> + struct wayland_output *output = to_wayland_output(base);
> >> + struct wayland_parent_output *poutput = output->user_data;
> >>
> >> if (poutput->current_mode) {
> >> - mode = poutput->current_mode;
> >> + output->poutput_mode = poutput->current_mode;
> >> } else if (poutput->preferred_mode) {
> >> - mode = poutput->preferred_mode;
> >> + output->poutput_mode = poutput->preferred_mode;
> >> } else if (!wl_list_empty(&poutput->mode_list)) {
> >> - mode = container_of(poutput->mode_list.next,
> >> - struct weston_mode, link);
> >> - } else {
> >> - weston_log("No valid modes found. Skipping output\n");
> >> - return NULL;
> >> - }
> >> -
> >> - if (!wl_list_empty(&b->compositor->output_list)) {
> >> - output = container_of(b->compositor->output_list.prev,
> >> - struct wayland_output, base.link);
> >> - x = output->base.x + output->base.current_mode->width;
> >> + output->poutput_mode = container_of(poutput->mode_list.next,
> >> + struct weston_mode, link);
> >> } else {
> >> - x = 0;
> >> + weston_log("No valid modes found. Cannot configure an output.\n");
> >> + return -1;
> >> }
> >>
> >> - output = wayland_output_create(b, x, 0, mode->width, mode->height,
> >> - NULL, 0,
> >> - WL_OUTPUT_TRANSFORM_NORMAL, 1);
> >> - if (!output)
> >> - return NULL;
> >> + if (wayland_output_configure(&output->base, output->poutput_mode->width,
> >> + output->poutput_mode->height) < 0)
> >
> > If wayland_output_configure() multiplies by scale and applies
> > transformation, then passing mode->width and mode->height directly here
> > is wrong.
> >
> > The "video mode" is given in output pixels, so it is already multiplied
> > by scale and rotated by transform.
> >
> > I see the old code was equally wrong, particularly if the output was
> > rotated 90 degrees it would create a window with inverted aspect ratio.
> > Ok, so if this keeps the old broken behaviour, it's fine.
> >
> > Testing the upstream x11-backend, it looks like we expect to put the
> > logical output size (in global coordinates) in weston.ini, and then
> > expect the scale and transform to be applied on that to get the output
> > resolution in output pixels.
> >
>
> This is for parent output (fs shell and sprawl). You can't change the size
> here. Scale and transform are also hardcoded to "scale = 1, transform = WL_OUTPUT_TRANSFORM_NORMAL"
> (well, they used to be and I let them remain that way).
>
> See:
>
> https://cgit.freedesktop.org/wayland/weston/tree/libweston/compositor-wayland.c#n1148
>
> and:
>
> https://github.com/krezovic/weston/blob/gsoc-v10/compositor/main.c#L1545
>
> Those values are set just for sake of it, it's not that anything uses them.
Yes, but ideally we might use also the scale and transform from the
parent output.
Those values are no longer hardcoded in this site here, which makes
things harder to follow, and wayland_output_configure() does use them.
However, that doesn't really affect this patch. Except maybe needing a
comment about scale and transform.
Btw. wayland_output_configure() forgets to apply transform, but that's
how it was already.
> >> + return -1;
> >>
> >> output->parent.output = poutput->global;
> >>
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160818/b8f223ba/attachment-0001.sig>
More information about the wayland-devel
mailing list