Re: [PATCH weston v2] clients: Add XKB compose key support
On Fri, Oct 07, 2016 at 12:56:00PM -0700, Bryce Harrington wrote:
> On Fri, Oct 07, 2016 at 02:48:41PM +0300, Ran Benita wrote:
> > > + /* Look up the appropriate locale, or use "C" as default */
> > > + locale = getenv("LC_ALL");
> > > + if (!locale)
> > > + locale = "C";
> >
> > Is there a reason why you decided not to use the "full" procedure, i.e.
> > also try LC_CTYPE and LANG?
>
> No particular reason. I did wonder if it would be worthwhile to check
> those too, but wasn't sure what order they should be checked. Should it
> be LANG first, then LC_CTYPE, and then fallback to LC_ALL?
I got the order from locale(7):
If the second argument to setlocale(3) is an empty string, "",
for the default locale, it is determined using the following
steps:
1. If there is a non-null environment variable LC_ALL, the
value of LC_ALL is used.
2. If an environment variable with the same name as one of
the categories above exists and is non-null, its value is
used for that category.
3. If there is a non-null environment variable LANG, the
value of LANG is used.
at step 2, the relevant category in this case is LC_CTYPE. So
LC_ALL -> LC_CTYPE -> LANG -> "C".
Ran
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
On Fri, Oct 07, 2016 at 02:48:41PM +0300, Ran Benita wrote:
> > + /* Look up the appropriate locale, or use "C" as default */
> > + locale = getenv("LC_ALL");
> > + if (!locale)
> > + locale = "C";
>
> Is there a reason why you decided not to use the "full" procedure, i.e.
> also try LC_CTYPE and LANG?
No particular reason. I did wonder if it would be worthwhile to check
those too, but wasn't sure what order they should be checked. Should it
be LANG first, then LC_CTYPE, and then fallback to LC_ALL?
> > + /* Set up XKB compose table */
> > + compose_table =
> > xkb_compose_table_new_from_locale(input->display->xkb_context,
> > + locale,
> > +
> > XKB_COMPOSE_COMPILE_NO_FLAGS);
> > + if (!compose_table) {
> > + fprintf(stderr, "could not create XKB compose table for locale
> > '%s'\n", locale);
> > + xkb_state_unref(state);
> > + xkb_keymap_unref(keymap);
> > + return;
> > + }
>
> In my opinion, it would be better to proceed without compose, than to
> fail the entire setup. If, for example, the compose files are not
> available, then it's OK to just skip it. The alternative is that the
> window does not show up at all (I assume).
>
> Ran
Ok, good point.
Bryce
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[weston] gl-renderer: conditionally call query_buffer while gl_renderer_attach
While gl_renderer_attach, query_buffer should be call only if the query_buffer function exists. Signed-off-by: Vincent Abriou --- libweston/gl-renderer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c index 6276188..caa72d7 100644 --- a/libweston/gl-renderer.c +++ b/libweston/gl-renderer.c @@ -2004,7 +2004,8 @@ gl_renderer_attach(struct weston_surface *es, struct weston_buffer *buffer) if (shm_buffer) gl_renderer_attach_shm(es, buffer, shm_buffer); - else if (gr->query_buffer(gr->egl_display, (void *) buffer->resource, + else if (gr->query_buffer && +gr->query_buffer(gr->egl_display, (void *)buffer->resource, EGL_TEXTURE_FORMAT, &format)) gl_renderer_attach_egl(es, buffer, format); else if ((dmabuf = linux_dmabuf_buffer_get(buffer->resource))) -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
Even if EGL_EXT_image_dma_buf_import does not provide a way to query
the supported pixel formats, we can at least expose to the DMABUF
protocol the pixel formats already explicitly supported as fallback
by gl-renderer in import_yuv_dmabuf function plus the standard RGB formats
that all GPU supports:
ARGB, XRGB, RGB565, YUYV, NV12, YUV420 and YUV444.
Signed-off-by: Vincent Abriou
---
Makefile.am | 3 +++
libweston/linux-dmabuf.c | 18 +++---
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index c94c211..6366c6f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -72,6 +72,9 @@ install-libweston_moduleLTLIBRARIES
install-moduleLTLIBRARIES: install-libLTLIBR
lib_LTLIBRARIES = libweston-@LIBWESTON_MAJOR@.la
libweston_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
libweston_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
$(LIBUNWIND_CFLAGS)
+if ENABLE_DRM_COMPOSITOR
+libweston_@LIBWESTON_MAJOR@_la_CFLAGS += $(DRM_COMPOSITOR_CFLAGS)
+endif
libweston_@LIBWESTON_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
$(LIBINPUT_BACKEND_LIBS) libshared.la
diff --git a/libweston/linux-dmabuf.c b/libweston/linux-dmabuf.c
index 7b29f08..c288333 100644
--- a/libweston/linux-dmabuf.c
+++ b/libweston/linux-dmabuf.c
@@ -23,6 +23,7 @@
#include "config.h"
#include
+#include
#include
#include
#include
@@ -30,6 +31,17 @@
#include "compositor.h"
#include "linux-dmabuf.h"
#include "linux-dmabuf-unstable-v1-server-protocol.h"
+#include "shared/helpers.h"
+
+uint32_t dmabuf_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YUV444,
+};
static void
linux_dmabuf_buffer_destroy(struct linux_dmabuf_buffer *buffer)
@@ -423,6 +435,7 @@ bind_linux_dmabuf(struct wl_client *client,
{
struct weston_compositor *compositor = data;
struct wl_resource *resource;
+ unsigned int i;
resource = wl_resource_create(client, &zwp_linux_dmabuf_v1_interface,
version, id);
@@ -434,9 +447,8 @@ bind_linux_dmabuf(struct wl_client *client,
wl_resource_set_implementation(resource, &linux_dmabuf_implementation,
compositor, NULL);
- /* EGL_EXT_image_dma_buf_import does not provide a way to query the
-* supported pixel formats. */
- /* XXX: send formats */
+ for (i = 0; i < ARRAY_LENGTH(dmabuf_formats); i++)
+ zwp_linux_dmabuf_v1_send_format(resource, dmabuf_formats[i]);
}
/** Advertise linux_dmabuf support
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
> + /* Look up the appropriate locale, or use "C" as default */
> + locale = getenv("LC_ALL");
> + if (!locale)
> + locale = "C";
Is there a reason why you decided not to use the "full" procedure, i.e.
also try LC_CTYPE and LANG?
> + /* Set up XKB compose table */
> + compose_table =
> xkb_compose_table_new_from_locale(input->display->xkb_context,
> + locale,
> +
> XKB_COMPOSE_COMPILE_NO_FLAGS);
> + if (!compose_table) {
> + fprintf(stderr, "could not create XKB compose table for locale
> '%s'\n", locale);
> + xkb_state_unref(state);
> + xkb_keymap_unref(keymap);
> + return;
> + }
In my opinion, it would be better to proceed without compose, than to
fail the entire setup. If, for example, the compose files are not
available, then it's OK to just skip it. The alternative is that the
window does not show up at all (I assume).
Ran
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/4] compositor: Implement horizontal output layout configuration
On Fri, 30 Sep 2016 23:25:30 +0200
Armin Krezović wrote:
> This patch adds horizontal output layout configuration using
> the previously added code.
>
> When an output is added, it looks for outputs that it should
> be placed next to, if they are specified. If such output is
> found, output layout is updated if needed (outputs on a given
> position are moved right if they collide with the new output)
> and the output is positioned next to the specified output,
> either on its left or its right.
>
> If a certain position wasn't specified for the current output,
> the compositor looks for any existing outputs that have
> current output specified on that position, and updates the
> positions accordingly, if corresponding output is found.
>
> Output positions can only be set once, where config file
> specified option is always set, even if such output will
> never be present. If a certain position wasn't set, the
> compositor may update the position if it finds another
> output that wants to be placed next to the output that's
> being configured. In such case, first existing output that
> matches the position is picked up, rest is ignored.
>
> Default behaviour is still to place outputs that have
> no matching configuration top right.
>
> Signed-off-by: Armin Krezović
> ---
> compositor/main.c | 159
> +++---
> 1 file changed, 153 insertions(+), 6 deletions(-)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index e379d8d..c039ae6 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -480,20 +480,167 @@ wet_init_parsed_options(struct weston_compositor *ec)
> return config;
> }
>
> +static struct wet_output *
> +wet_output_from_output(struct wet_compositor *compositor, struct
> weston_output *output)
> +{
> + struct wet_output *iterator, *next;
> +
> + if (wl_list_empty(&compositor->output_layout_list))
> + return NULL;
> +
> + wl_list_for_each_safe(iterator, next, &compositor->output_layout_list,
> link) {
> + if (iterator->output == output)
> + return iterator;
> + }
Hi,
wet_output uses a destroy listener on the output, so use that:
listener = wl_signal_get(&output->destroy_signal, handle_output_destroy);
wet_output = container_of(...);
> +
> + return NULL;
> +}
> +
> +static struct wet_output *
> +wet_output_from_name(struct wet_compositor *compositor, const char *name)
> +{
> + struct wet_output *iterator, *next;
> +
> + if (wl_list_empty(&compositor->output_layout_list))
> + return NULL;
> +
> + wl_list_for_each_safe(iterator, next, &compositor->output_layout_list,
> link) {
> + if (!strcmp(iterator->output->name, name))
> + return iterator;
> + }
No need to check for empty list, and no need to use _safe. Applies
probably to all loop below, too.
> +
> + return NULL;
> +}
> +
I kind of wonder how much sense it makes to do first just 1-D layout
and then later expand to 2-D layout. I fear the code re-use might be
non-existant.
If you are struggling with 2-D layout, maybe X.org could provide some
example?
I do not have a suggestion for a good layout algorithm or even a method
of description. People have been configuring X.org for ages, so
something similar might be familiar.
Positioning by absolute coordinates I would like to leave out if
possible, or at least you would need to check that:
- all outputs create a single connected region (so pointers can travel
everywhere)
- no outputs overlap (IIRC this is a limitation of Weston's damage
tracking)
Thanks,
pq
pgpipaRSR54of.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/4] compositor: Add internal output object used for layout configuration
On Fri, 30 Sep 2016 23:25:29 +0200
Armin Krezović wrote:
> This adds weston specific output object, which contains information
> about output's position, relative to other outputs. DRM and Windowed
> backends code has been updated to make use of the new code and
> parse new configuration options for a given output, if it was
> specified.
>
> New configuration file options for [output] section have been added:
>
> left-of: Specifies which output should be on the right side of the
> current output.
>
> right-of: Specifies which output should be on the left side of the
> current output.
>
> Signed-off-by: Armin Krezović
> ---
> compositor/main.c | 81
> +++
> 1 file changed, 81 insertions(+)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index 503016e..e379d8d 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -78,6 +78,18 @@ struct wet_compositor {
> struct weston_config *config;
> struct wet_output_config *parsed_options;
> struct wl_listener pending_output_listener;
> + struct wl_list output_layout_list;
> +};
> +
> +struct wet_output {
> + struct weston_output *output;
> + struct wl_listener output_destroy_listener;
> + struct wl_list link;
> +
> + char *left_output_name;
> + char *right_output_name;
Hi
Rather than linking by name, would it not be easier to link by pointer
to struct wet_output?
That means when you encounter a directive like "left-of=F5" and there
is no output F5 yet, you would create a new struct wet_output for F5
but leave the weston_output pointer NULL, to be filled out if F5
actually appears.
That means you would always have a graph in memory instead of doing
searches to find if output of this name actually exists. I'm not sure
if that's actually simpler in the end.
Having the complete graph would allow you to "jump over" outputs that
do not exist at the moment: e.g. order F1 -> (F2) -> F3.
> +
> + bool position_set;
> };
>
> + weston_config_section_get_string(section, "left-of", &out, NULL);
> + wet_output->right_output_name = out ? strdup(out) : NULL;
> + free(out);
> +
> + weston_config_section_get_string(section, "right-of", &out, NULL);
> + wet_output->left_output_name = out ? strdup(out) : NULL;
> + free(out);
Yes, this looks like an intuitive approach, storing the relationship in
inverse compared to how users write it out in weston.ini.
Thanks,
pq
pgpOqdM0WjihR.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/4] weston: Move output position setting to compositor
On Fri, 30 Sep 2016 23:25:28 +0200 Armin Krezović wrote: > This moves current output positioning code and scale/transform > application to the compositor itself, so the compositor > can configure output layouts any way it wants. > > A helper function for setting x and y coordinates is also > added, and couple of assertions to weston_output_enable() > as well, to make sure everything has been set up. > > Signed-off-by: Armin Krezović > --- > compositor/main.c | 26 ++ > libweston/compositor.c | 40 ++-- > libweston/compositor.h | 3 +++ > 3 files changed, 51 insertions(+), 18 deletions(-) Hi, moving the output positioning out from libweston and into the compositor is very good. The setter for x,y is good too. I'm just not sure we should assume that outputs cannot occupy the negative coordinates. If one hotplugs an output to the left, I'd assume the existing windows would stay put on the right. If we cannot use negative coordinates, everything has to be moved to keep them at the same position from the user point of view. Thanks, pq pgp1Wq3Nq03Vq.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
Hi,
On 7 October 2016 at 05:24, Bryce Harrington wrote:
> @@ -3016,8 +3024,38 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
> *keyboard,
> return;
> }
>
> + /* Look up the appropriate locale, or use "C" as default */
> + locale = getenv("LC_ALL");
> + if (!locale)
> + locale = "C";
> +
> + /* Set up XKB compose table */
> + compose_table =
> xkb_compose_table_new_from_locale(input->display->xkb_context,
> + locale,
> +
> XKB_COMPOSE_COMPILE_NO_FLAGS);
This line is quite long. You can get it to exactly 80 characters like
so (this style is quite common within Weston):
compose_table =
xkb_compose_table_new_from_locale(input->display->xkb_context,
locale,
XKB_COMPOSE_COMPILE_NO_FLAGS);
> +/* Translate symbols appropriately if a compose sequence is being entered */
> +static xkb_keysym_t
> +process_key_press(xkb_keysym_t sym, struct input *input)
> +{
> + if (sym == XKB_KEY_NoSymbol)
> + return sym;
> + if (xkb_compose_state_feed(input->xkb.compose_state, sym) !=
> XKB_COMPOSE_FEED_ACCEPTED)
> + return sym;
Same here.
> + switch (xkb_compose_state_get_status(input->xkb.compose_state)) {
> + case XKB_COMPOSE_COMPOSING:
> + return XKB_KEY_NoSymbol;
> + case XKB_COMPOSE_COMPOSED:
> + return
> xkb_compose_state_get_one_sym(input->xkb.compose_state);
> + case XKB_COMPOSE_CANCELLED:
> + return XKB_KEY_NoSymbol;
> + case XKB_COMPOSE_NOTHING:
> + return sym;
> + }
> +
> + return sym;
This line is unreachable, and could also be handled with a 'default' case.
The rest looks good to me though, and I like the split function, so
with those fixed:
Reviewed-by: Daniel Stone
Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/4] libweston: Export weston_output_transform_scale_init
On Fri, 30 Sep 2016 23:25:27 +0200
Armin Krezović wrote:
> This is required for implementing output layout setting
> which relies on current output width and height, and
> those are calculated in this function.
>
> It also changes the function signature to make use
> of already stored scale and transform values in the
> weston_output object.
>
> Signed-off-by: Armin Krezović
> ---
> libweston/compositor.c | 31 ++-
> libweston/compositor.h | 2 ++
> 2 files changed, 20 insertions(+), 13 deletions(-)
Hi,
I think this would need a bit more thought on the API.
The requirement is that one has to set some output parameters (video
mode, transform, scale) before the output size is available. Output
size then is needed for doing the layout.
IMO it would make more sense to add a new function
weston_output_get_size(), which would take into account the currently
set (pending) parameters and compute the resulting size.
This is also good in the long term since we would like to make struct
weston_output opaque to the libweston user. We are going to need a
getter for the size anyway.
Thanks,
pq
>
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index d552e18..7ebc08c 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -67,10 +67,6 @@
> #define DEFAULT_REPAINT_WINDOW 7 /* milliseconds */
>
> static void
> -weston_output_transform_scale_init(struct weston_output *output,
> -uint32_t transform, uint32_t scale);
> -
> -static void
> weston_compositor_build_view_list(struct weston_compositor *compositor);
>
> static void weston_mode_switch_finish(struct weston_output *output,
> @@ -86,7 +82,7 @@ static void weston_mode_switch_finish(struct weston_output
> *output,
> pixman_region32_copy(&old_output_region, &output->region);
>
> /* Update output region and transformation matrix */
> - weston_output_transform_scale_init(output, output->transform,
> output->current_scale);
> + weston_output_transform_scale_init(output);
>
> pixman_region32_init(&output->previous_damage);
> pixman_region32_init_rect(&output->region, output->x, output->y,
> @@ -4212,17 +4208,25 @@ weston_output_update_matrix(struct weston_output
> *output)
> weston_matrix_invert(&output->inverse_matrix, &output->matrix);
> }
>
> -static void
> -weston_output_transform_scale_init(struct weston_output *output, uint32_t
> transform, uint32_t scale)
> +WL_EXPORT void
> +weston_output_transform_scale_init(struct weston_output *output)
> {
> - output->transform = transform;
> - output->native_scale = scale;
> - output->current_scale = scale;
> + /* Make sure transform and scale are set */
> + assert(output->scale);
> + assert(output->transform != UINT32_MAX);
> +
> + /* Make sure output->current_mode has been constructed. */
> + assert(output->current_mode);
> +
> + /* TODO: Use output->original_scale in weston_output_set_scale() */
> + output->original_scale = output->scale;
> + output->native_scale = output->scale;
> + output->current_scale = output->scale;
>
> convert_size_by_transform_scale(&output->width, &output->height,
> output->current_mode->width,
> output->current_mode->height,
> - transform, scale);
> + output->transform, output->scale);
> }
>
> static void
> @@ -4448,6 +4452,8 @@ weston_output_init(struct weston_output *output,
> output->scale = 0;
> /* Can't use -1 on uint32_t and 0 is valid enum value */
> output->transform = UINT32_MAX;
> +
> + output->current_mode = NULL;
> }
>
> /** Adds weston_output object to pending output list.
> @@ -4529,9 +4535,8 @@ weston_output_enable(struct weston_output *output)
> output->x = x;
> output->y = y;
> output->dirty = 1;
> - output->original_scale = output->scale;
>
> - weston_output_transform_scale_init(output, output->transform,
> output->scale);
> + weston_output_transform_scale_init(output);
> weston_output_init_zoom(output);
>
> weston_output_init_geometry(output, x, y);
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 3e486d5..a39b327 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1596,6 +1596,8 @@ weston_output_activate_zoom(struct weston_output
> *output,
> void
> weston_output_update_matrix(struct weston_output *output);
> void
> +weston_output_transform_scale_init(struct weston_output *output);
> +void
> weston_output_move(struct weston_output *output, int x, int y);
>
> void
pgpoM1NJnvSdr.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
