Re: [PATCH weston v5 03/11] drm: port the drm backend to the new init api
2016-04-16 1:02 GMT+03:00 Bryce Harrington : > On Fri, Apr 15, 2016 at 02:32:04PM +0200, Benoit Gschwind wrote: >> > + /** The seat to be used by the output. Set to NULL to use the >> > +* default seat. */ >> > + char *seat; >> > + /** The modeline to be used by the output. Refer to the documentation >> > +* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */ >> > + char *modeline; >> >> Maybe enum with drmModeModeInfo* is better API than free string API? > > I think you're probably right that weston_drm_backend_output_config > could hold a pointer to the enum rather than the string. I've moved > weston_drm_backend_output_config to be a private struct in > compositor-drm.c so this should be doable. One of the earlier revisions had that but after a discussion with Quentin we decided to put the parsing in the backend rather than every compositor. Cheers, Giulio > > But I think I'm going to opt to leave this change to Giulio as follow-up > work, as I think it needs deeper thought than I'm going to be able to > give it in this patchset. There was also some discussion about whether > WESTON_DRM_BACKEND_OUTPUT_PREFERRED should be renamed, in which case it > would make sense to do the change in conjunction with that. > > Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v5 00/11] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 03:17:15PM +0200, Benoit Gschwind wrote: > Hello Bryce, > > The patches set was tested above [1] > > I tested drm-bakcend and x11-backend successfully with the full patchset. > > Here are the resume of my comments and review. I send separate e-mail > when contextual comments are required. Thanks for reviewing it, Benoit! Unfortunately I forgot to include your R-b's but I'm pretty certain there will be a v7 of this set, and will include them there. Still, due to how extensive the v6 changes were, another re-review would be worth the effort. Bryce > PATCH 01/11: > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 02/11: > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 03/11 > > Already commented in another e-mail > > PATCH 04/11 > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 05/11 > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 06/11 > > Agree with Pekka comments, I'm available to fix issues if needed. > > PATCH 07/11 > > Agree with Pekka comments, I'm available to fix issues if needed. > > PATCH 08/11 > > The re-factor look functional. Other decision tie to the community :) > > PATCH 09/11 > > This patch look to address some concern I raised in the patch 03/11, but > seems to need more discussion. > > PATCH 10/11 > > The patch look to address issue of PATCH 07/11 that Pekka raised about > freeing config variable. > > PATCH 11/11 > > Look fine. > > Reviewed-by: Benoit Gschwind > > > > [1] git node: 94cb06a208130b0ee16553a2cd513e5e7d67f368 > > > > Le 13/04/2016 12:25, Bryce Harrington a écrit : > > In following up on my earlier update of Giulio's drm backend config > > patch, I've taken the liberty to try and also integrate a couple of > > Benoit's other backend configuration patches into this patchset. > > > > Giulio and Benoit took different approaches in their implementations. > > I've attempted to reconcile them so that they are stylistically > > consistent, along with incorporating pq's request to have the root > > config structure incorporate versioning information. I went through all > > the review comments against each of the backend config patches and > > incorporated the suggestions people made. The resultant changes have > > ended up a bit more extensive than I had expected, and I apologize for > > any toe stepping I'm doing but my hope is to move the collective work > > closer to being landable. > > > > --- > > todo: > > - Use backend-specific header #defines for struct_version values > > v5: > > - Add missing compositor-drm.h > > - Integrate wayland, x11, and headless backend patches > > - Implement struct versioning for wayland, x11, and headless backend > > patches in the weston_backend_config structure, as requested by pq > > - Drop bzero usage as suggested in review by Jan Engelhardt > > - Don't change 'backend_init' entry point function name, as suggested > > in review by Giulio > > - Prefer use of load_backend_new() for actual module loading, as > > suggested in Giulio's review of the x11 backend patch > > - Refactor all backend initialization code paths and code style for > > consistency. > > - Squashed drm config struct versioning with drm patch; remainder moved > > to a prerequisite patch. > > v4: > > - Update to current trunk > > - Add missing param doc for mode in drm_output_choose_initial_mode > > - Rebase to account for code changes by 91880f1e to make vt > > switching configurable. > > > > > > > > Benoit Gschwind (1): > > x11: port the x11 backend to the new init api > > > > Bryce Harrington (9): > > Revert "main: Remove unused function load_backend_new()" > > compositor: Version the backend configuration structures > > drm: Fix gcc warning about missing braces. > > compositor: Document refs for alternatives/assumptions for backend > > configs > > headless: port the headless backend to the new init api > > drm: Code and comments reformatting for consistency with other backend > > configs > > drm: Don't hang onto the backend config object post-backend_init > > Enforce destruction of all backend config objects after initialization > > drm: Drop use of drm_config config wrapper > > > > Giulio Camuffo (1): > > drm: port the drm backend to the new init api > > > > Makefile.am | 5 + > > src/compositor-drm.c | 216 ++-- > > src/compositor-drm.h | 125 +++ > > src/compositor-headless.c | 69 +-- > > src/compositor-headless.h | 51 > > src/compositor-x11.c | 157 > > src/compositor-x11.h | 60 + > > src/compositor.h | 24 +++- > > src/main.c| 304 > > +- > > 9 files changed, 734 insertions(+), 277 deletions(-) > > create mode 100644 src/compositor-drm.h > >
Re: [PATCH weston v5 00/11] drm: port the drm backend to the new init api
On Wed, Apr 13, 2016 at 05:21:39PM +0300, Pekka Paalanen wrote: > On Wed, 13 Apr 2016 03:25:04 -0700 > Bryce Harrington wrote: > > > In following up on my earlier update of Giulio's drm backend config > > patch, I've taken the liberty to try and also integrate a couple of > > Benoit's other backend configuration patches into this patchset. > > > > Giulio and Benoit took different approaches in their implementations. > > I've attempted to reconcile them so that they are stylistically > > consistent, along with incorporating pq's request to have the root > > config structure incorporate versioning information. I went through all > > the review comments against each of the backend config patches and > > incorporated the suggestions people made. The resultant changes have > > ended up a bit more extensive than I had expected, and I apologize for > > any toe stepping I'm doing but my hope is to move the collective work > > closer to being landable. > > > > --- > > todo: > > - Use backend-specific header #defines for struct_version values > > v5: > > - Add missing compositor-drm.h > > - Integrate wayland, x11, and headless backend patches > > - Implement struct versioning for wayland, x11, and headless backend > > patches in the weston_backend_config structure, as requested by pq > > - Drop bzero usage as suggested in review by Jan Engelhardt > > - Don't change 'backend_init' entry point function name, as suggested > > in review by Giulio > > - Prefer use of load_backend_new() for actual module loading, as > > suggested in Giulio's review of the x11 backend patch > > - Refactor all backend initialization code paths and code style for > > consistency. > > - Squashed drm config struct versioning with drm patch; remainder moved > > to a prerequisite patch. > > v4: > > - Update to current trunk > > - Add missing param doc for mode in drm_output_choose_initial_mode > > - Rebase to account for code changes by 91880f1e to make vt > > switching configurable. > > > > > > > > Benoit Gschwind (1): > > x11: port the x11 backend to the new init api > > > > Bryce Harrington (9): > > Revert "main: Remove unused function load_backend_new()" > > compositor: Version the backend configuration structures > > drm: Fix gcc warning about missing braces. > > compositor: Document refs for alternatives/assumptions for backend > > configs > > headless: port the headless backend to the new init api > > drm: Code and comments reformatting for consistency with other backend > > configs > > drm: Don't hang onto the backend config object post-backend_init > > Enforce destruction of all backend config objects after initialization > > drm: Drop use of drm_config config wrapper > > > > Giulio Camuffo (1): > > drm: port the drm backend to the new init api > > > > Makefile.am | 5 + > > src/compositor-drm.c | 216 ++-- > > src/compositor-drm.h | 125 +++ > > src/compositor-headless.c | 69 +-- > > src/compositor-headless.h | 51 > > src/compositor-x11.c | 157 > > src/compositor-x11.h | 60 + > > src/compositor.h | 24 +++- > > src/main.c| 304 > > +- > > 9 files changed, 734 insertions(+), 277 deletions(-) > > create mode 100644 src/compositor-drm.h > > create mode 100644 src/compositor-headless.h > > create mode 100644 src/compositor-x11.h > > > > Hi, > > patches: 1, 2, 4, 9, 10 > are Reviewed-by: Pekka Paalanen > > Patch 3 already has my Acked-by and it is good to go, since the patches > coming later fix the issues I raised earlier. > > Patch 5 seems to have dropped the email archive links, those would be > good to keep. With them added back, R-b me. > > I think patches 8-11 should have come before 6 and 7. I needed to > review 8-11 first to see where design ended up, before I can check that > 6 and 7 follow it too. > > Seems like the changes in patch 10 should be squashed into the patches > converting each backend, but okay anyway. > > Patch 11 is R-b me otherwise, except it will need some changes > according to the comment to patch 8. > > I am still in the middle of reviewing patches 6 and 7. Patch 6 has some > compositor-drm change mixed in. More comments for these two patches > will follow another day. Thanks for reviewing the patchset (and thanks Giulio and Benoit as well). Except for one or two items I'm leaving for followup I think I've incorporated all the requested changes, but I wouldn't be at all surprised if I missed something, so apologies ahead of time. With all the rebasing I could have also introduced an irregularity or two. So even though I've included your R-b's, you may want to take another look at the patches and doublecheck. Re-testing of all three backends would also be appreciated; I've verified make check works bu
[PATCH weston v6 01/12] drm: Spelling fix in comment
Signed-off-by: Bryce Harrington --- src/compositor-drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 119b6c5..a47b453 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -1551,7 +1551,7 @@ create_gbm_device(int fd) } /* When initializing EGL, if the preferred buffer format isn't available - * we may be able to susbstitute an ARGB format for an XRGB one. + * we may be able to substitute an ARGB format for an XRGB one. * * This returns 0 if substitution isn't possible, but 0 might be a * legitimate format for other EGL platforms, so the caller is -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 09/12] drm: Move drm's output configuration into the drm backend
Signed-off-by: Bryce Harrington
---
src/compositor-drm.c | 93
src/compositor-drm.h | 41 ---
src/main.c | 43
3 files changed, 80 insertions(+), 97 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 6ef706a..d129adc 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -75,6 +75,41 @@
#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
#endif
+enum weston_drm_backend_output_mode {
+ /** The output is disabled */
+ WESTON_DRM_BACKEND_OUTPUT_OFF,
+
+ /** The output will use the current active mode */
+ WESTON_DRM_BACKEND_OUTPUT_CURRENT,
+
+ /** The output will use the preferred mode. A modeline can be provided
+* by setting weston_backend_output_config::modeline in the form of
+* "WIDTHxHEIGHT" or in the form of an explicit modeline calculated
+* using e.g. the cvt tool. If a valid modeline is supplied it will be
+* used, if invalid or NULL the preferred available mode will be used.
*/
+ WESTON_DRM_BACKEND_OUTPUT_PREFERRED,
+};
+
+struct weston_drm_backend_output_config {
+ struct weston_backend_output_config base;
+
+ /** The pixel format to be used by the output. Valid values are:
+* - NULL - The format set at backend creation time will be used
+* - "xrgb"
+* - "rgb565"
+* - "xrgb2101010"
+*/
+ char *gbm_format;
+
+ /** The seat to be used by the output. Set to NULL to use the
+* default seat. */
+ char *seat;
+
+ /** The modeline to be used by the output. Refer to the documentation
+* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */
+ char *modeline;
+};
+
struct drm_backend {
struct weston_backend base;
struct weston_compositor *compositor;
@@ -121,16 +156,6 @@ struct drm_backend {
int32_t cursor_width;
int32_t cursor_height;
-/** Callback used to configure the outputs.
-*
- * This function will be called by the backend when a new DRM
- * output needs to be configured.
- */
-enum weston_drm_backend_output_mode
- (*configure_output)(struct weston_compositor *compositor,
- bool use_current_mode,
- const char *name,
- struct weston_drm_backend_output_config
*output_config);
bool use_current_mode;
};
@@ -2275,6 +2300,49 @@ connector_get_current_mode(drmModeConnector *connector,
int drm_fd,
return 0;
}
+static enum weston_drm_backend_output_mode
+drm_configure_output(struct weston_compositor *c,
+ bool use_current_mode,
+ const char *name,
+ struct weston_drm_backend_output_config *config)
+{
+ struct weston_config *wc = weston_compositor_get_user_data(c);
+ struct weston_config_section *section;
+ char *s;
+ int scale;
+ enum weston_drm_backend_output_mode mode =
+ WESTON_DRM_BACKEND_OUTPUT_PREFERRED;
+
+ section = weston_config_get_section(wc, "output", "name", name);
+ weston_config_section_get_string(section, "mode", &s, "preferred");
+ if (strcmp(s, "off") == 0) {
+ free(s);
+ return WESTON_DRM_BACKEND_OUTPUT_OFF;
+ }
+
+ if (use_current_mode || strcmp(s, "current") == 0) {
+ mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
+ } else if (strcmp(s, "preferred") != 0) {
+ config->modeline = s;
+ s = NULL;
+ }
+ free(s);
+
+ weston_config_section_get_int(section, "scale", &scale, 1);
+ config->base.scale = scale >= 1 ? scale : 1;
+ weston_config_section_get_string(section, "transform", &s, "normal");
+ if (weston_parse_transform(s, &config->base.transform) < 0)
+ weston_log("Invalid transform \"%s\" for output %s\n",
+ s, name);
+ free(s);
+
+ weston_config_section_get_string(section,
+"gbm-format", &config->gbm_format,
NULL);
+ weston_config_section_get_string(section, "seat", &config->seat, "");
+ return mode;
+}
+
+
/**
* Create and configure a Weston output structure
*
@@ -2321,8 +2389,8 @@ create_output_for_connector(struct drm_backend *b,
output->base.serial_number = "unknown";
wl_list_init(&output->base.mode_list);
- mode = b->configure_output(b->compositor, b->use_current_mode,
- output->base.name, &config);
+ mode = drm_configure_output(b->compositor, b->use_current_mode,
+ output->base.name, &config);
if (parse_gbm_format(config.gbm_format, b->gbm_format,
&output->gbm_format) == -1)
output->gbm_format = b->gbm_format;
@@ -3059,7
[PATCH weston v6 05/12] drm: port the drm backend to the new init api
From: Giulio Camuffo
Signed-off-by: Bryce Harrington
Reviewed-by: Quentin Glidic
Acked-by: Pekka Paalanen
Tested-by: Benoit Gschwind
v6:
- Fix gcc warning about missing braces. Use double-brackets for config
initializer for create_output_for_connector to avoid gcc warning,
since first element is another struct. (See GCC bug 53119.)
- Code and comments reformatting for consistency with other backend
configs
- Don't use underscore prefix in header guard
- Add stub config_init_to_defaults()
- Define the version number in the header
- Allocate config on stack
v5:
- Update to reflect format rename to gdb_format
- Initialize width/height (suggested by pq)
- squash drm structure versioning (suggested by pq)
Signed-off-by: Bryce Harrington
---
Makefile.am | 3 +
src/compositor-drm.c | 212 +--
src/compositor-drm.h | 126 ++
src/compositor.h | 2 -
src/main.c | 97 ++-
5 files changed, 311 insertions(+), 129 deletions(-)
create mode 100644 src/compositor-drm.h
diff --git a/Makefile.am b/Makefile.am
index 9bed32c..eeb40fb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -72,6 +72,7 @@ weston_SOURCES = \
src/log.c \
src/compositor.c\
src/compositor.h\
+ src/compositor-drm.h\
src/input.c \
src/data-device.c \
src/screenshooter.c \
@@ -208,6 +209,7 @@ westonincludedir = $(includedir)/weston
westoninclude_HEADERS =\
src/version.h \
src/compositor.h\
+ src/compositor-drm.h\
src/timeline-object.h \
shared/matrix.h \
shared/config-parser.h \
@@ -277,6 +279,7 @@ drm_backend_la_CFLAGS = \
$(AM_CFLAGS)
drm_backend_la_SOURCES = \
src/compositor-drm.c\
+ src/compositor-drm.h\
$(INPUT_BACKEND_SOURCES)\
shared/helpers.h\
shared/timespec-util.h \
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index a47b453..2384fd2 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -46,12 +46,13 @@
#include
#include
+#include "compositor.h"
+#include "compositor-drm.h"
#include "shared/helpers.h"
#include "shared/timespec-util.h"
-#include "libbacklight.h"
-#include "compositor.h"
#include "gl-renderer.h"
#include "pixman-renderer.h"
+#include "libbacklight.h"
#include "libinput-seat.h"
#include "launcher-util.h"
#include "vaapi-recorder.h"
@@ -74,17 +75,6 @@
#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
#endif
-static int option_current_mode = 0;
-
-enum output_config {
- OUTPUT_CONFIG_INVALID = 0,
- OUTPUT_CONFIG_OFF,
- OUTPUT_CONFIG_PREFERRED,
- OUTPUT_CONFIG_CURRENT,
- OUTPUT_CONFIG_MODE,
- OUTPUT_CONFIG_MODELINE
-};
-
struct drm_backend {
struct weston_backend base;
struct weston_compositor *compositor;
@@ -130,6 +120,8 @@ struct drm_backend {
int32_t cursor_width;
int32_t cursor_height;
+
+ struct weston_drm_backend_config *config;
};
struct drm_mode {
@@ -220,13 +212,6 @@ struct drm_sprite {
uint32_t formats[];
};
-struct drm_parameters {
- int connector;
- int tty;
- int use_pixman;
- const char *seat_id;
-};
-
static struct gl_renderer_interface *gl_renderer;
static const char default_seat[] = "seat0";
@@ -2151,31 +2136,23 @@ setup_output_seat_constraint(struct drm_backend *b,
}
static int
-get_gbm_format_from_section(struct weston_config_section *section,
- uint32_t default_value,
- uint32_t *format)
+parse_gbm_format(const char *s, uint32_t default_value, uint32_t *gbm_format)
{
- char *s;
int ret = 0;
- weston_config_section_get_string(section,
-"gbm-format", &s, NULL);
-
if (s == NULL)
- *format = default_value;
+ *gbm_format = default_value;
else if (strcmp(s, "xrgb") == 0)
- *format = GBM_FORMAT_XRGB;
+ *gbm_format = GBM_FORMAT_XRGB;
else if (strcmp(s, "rgb565") == 0)
- *format = GBM_FORMAT_RGB565;
+ *gbm_format = GBM_FORMAT_RGB565;
else if (strcmp(s, "xrgb2101010") == 0)
- *format = GBM_FORMAT_XRGB2101010;
+ *gbm_format = GBM_FORMAT_XRGB2101010;
else {
[PATCH weston v6 08/12] drm: Don't hang onto the backend config object post-backend_init
The drm backend was copying most everything out of the config object
already, but now also copy the use_current_mode parameter and the
config_output function pointer, so that there are no remaining
references to the config object passed into backend_init().
By decoupling the config struct to the backend, if in the future if the
structure definition changes in non-backwards compatible ways, then any
version compatibility adaptation will be limited to just the
backend_init() routine.
With the use_current_mode moved into the main config class, the
drm_config wrapper is redundant. Dropping it helps make the drm backend
config initialization more consistent with the other backends.
Signed-off-by: Bryce Harrington
Reviewed-by: Pekka Paalanen
v6:
- Drop use of drm_config config wrapper
Signed-off-by: Bryce Harrington
---
src/compositor-drm.c | 24 +++-
src/compositor-drm.h | 3 ++-
src/main.c | 47 +++
3 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 2384fd2..6ef706a 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -121,7 +121,17 @@ struct drm_backend {
int32_t cursor_width;
int32_t cursor_height;
- struct weston_drm_backend_config *config;
+/** Callback used to configure the outputs.
+*
+ * This function will be called by the backend when a new DRM
+ * output needs to be configured.
+ */
+enum weston_drm_backend_output_mode
+ (*configure_output)(struct weston_compositor *compositor,
+ bool use_current_mode,
+ const char *name,
+ struct weston_drm_backend_output_config
*output_config);
+ bool use_current_mode;
};
struct drm_mode {
@@ -2311,8 +2321,8 @@ create_output_for_connector(struct drm_backend *b,
output->base.serial_number = "unknown";
wl_list_init(&output->base.mode_list);
- mode = b->config->configure_output(b->compositor, b->config,
- output->base.name, &config);
+ mode = b->configure_output(b->compositor, b->use_current_mode,
+ output->base.name, &config);
if (parse_gbm_format(config.gbm_format, b->gbm_format,
&output->gbm_format) == -1)
output->gbm_format = b->gbm_format;
@@ -2699,11 +2709,6 @@ drm_destroy(struct weston_compositor *ec)
weston_launcher_destroy(ec->launcher);
close(b->drm.fd);
-
- free(b->config->gbm_format);
- free(b->config->seat_id);
- free(b->config);
-
free(b);
}
@@ -3054,7 +3059,8 @@ drm_backend_create(struct weston_compositor *compositor,
b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
- b->config = config;
+ b->configure_output = config->configure_output;
+ b->use_current_mode = config->use_current_mode;
if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB,
&b->gbm_format) < 0)
goto err_compositor;
diff --git a/src/compositor-drm.h b/src/compositor-drm.h
index fdf5154..3b2dc17 100644
--- a/src/compositor-drm.h
+++ b/src/compositor-drm.h
@@ -114,9 +114,10 @@ struct weston_drm_backend_config {
*/
enum weston_drm_backend_output_mode
(*configure_output)(struct weston_compositor *compositor,
- struct weston_drm_backend_config
*backend_config,
+ bool use_current_mode,
const char *name,
struct weston_drm_backend_output_config
*output_config);
+ bool use_current_mode;
};
#ifdef __cplusplus
diff --git a/src/main.c b/src/main.c
index feb967d..21b7f5c 100644
--- a/src/main.c
+++ b/src/main.c
@@ -674,19 +674,12 @@ load_backend_new(struct weston_compositor *compositor,
const char *backend,
return backend_init(compositor, NULL, NULL, NULL, config_base);
}
-
-struct drm_config {
- struct weston_drm_backend_config base;
- bool use_current_mode;
-};
-
static enum weston_drm_backend_output_mode
drm_configure_output(struct weston_compositor *c,
-struct weston_drm_backend_config *backend_config,
+bool use_current_mode,
const char *name,
struct weston_drm_backend_output_config *config)
{
- struct drm_config *drm_config = (struct drm_config *)backend_config;
struct weston_config *wc = weston_compositor_get_user_data(c);
struct weston_config_section *section;
char *s;
@@ -701,7 +694,7 @@ drm_configure_output(struct weston_compositor *c,
return WESTON_DRM_BACKEND_OUTPUT_OFF;
}
- if (drm_config->use_current_mode || strcmp(s, "current"
[PATCH weston v6 02/12] Revert 'main: Remove unused function load_backend_new()'
This reverts commit 5ffbfffaf7758c33791978516d0a1100773b85e2.
Restore load_backend_new() for use with libweston backend configuration.
Signed-off-by: Bryce Harrington
Reviewed-by: Pekka Paalanen
---
src/main.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/src/main.c b/src/main.c
index 1850fa6..43de354 100644
--- a/src/main.c
+++ b/src/main.c
@@ -653,6 +653,23 @@ load_backend_old(struct weston_compositor *compositor,
const char *backend,
return backend_init(compositor, argc, argv, wc, NULL);
}
+/* Temporary function to be replaced by weston_compositor_load_backend(). */
+static int
+load_backend_new(struct weston_compositor *compositor, const char *backend,
+struct weston_backend_config *config_base)
+{
+ int (*backend_init)(struct weston_compositor *c,
+ int *argc, char *argv[],
+ struct weston_config *config,
+ struct weston_backend_config *config_base);
+
+ backend_init = weston_load_module(backend, "backend_init");
+ if (!backend_init)
+ return -1;
+
+ return backend_init(compositor, NULL, NULL, NULL, config_base);
+}
+
static int
load_backend(struct weston_compositor *compositor, const char *backend,
int *argc, char **argv, struct weston_config *config)
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 03/12] compositor: Drop unneeded create_output callback
Signed-off-by: Bryce Harrington
---
src/compositor.h | 8
1 file changed, 8 deletions(-)
diff --git a/src/compositor.h b/src/compositor.h
index 0ba00be..5ca497c 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -691,14 +691,6 @@ struct weston_backend_config {
struct weston_backend {
void (*destroy)(struct weston_compositor *compositor);
void (*restore)(struct weston_compositor *compositor);
- /* vfunc to create a new output with a given name and config.
-* backends not supporting the functionality will set this
-* to NULL.
-*/
- struct weston_output *
- (*create_output)(struct weston_compositor *compositor,
-const char *name,
-struct weston_backend_output_config *config);
};
struct weston_compositor {
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 00/12] Port three backends to the new init api
In following up on my earlier update of Giulio's drm backend config patch, I've taken the liberty to try and also integrate Benoit's other backend configuration patches into this patchset, and reconcile them for stylistic and design consistency. This includes adding a versioning capability for the backend config structures. This patchset also incorporates feedback from a number of reviewers. The resultant changes have ended up a bit more extensive than I had expected, and I apologize for any toe stepping I'm doing but my hope is to move the collective work closer to being landable. --- v6: - Squash various already-reviewed patches - Fix installation of header files in makefile - Add #defines for WESTON_*_BACKEND_CONFIG_VERSION - Don't use underscore prefixes in header guards - Add config_init_to_default() stubs to setup backend config defaults - Drop unneeded create_output callback in weston_backend structure - Pass gbm_format as enum rather than string in drm output configuration - Allocate config objects on stack - Fail if invalid output width or height are given in x11 backend - Preserve priority for width/height cmdline options in x11 backend - Make the drm output configuration private to the drm backend, allowing gbm_format to be tracked as an enum rather than string - Allocate all outputs in one go for the x11 backend v5: - Add missing compositor-drm.h - Integrate wayland, x11, and headless backend patches - Implement struct versioning for wayland, x11, and headless backend patches in the weston_backend_config structure, as requested by pq - Drop bzero usage as suggested in review by Jan Engelhardt - Don't change 'backend_init' entry point function name, as suggested in review by Giulio - Prefer use of load_backend_new() for actual module loading, as suggested in Giulio's review of the x11 backend patch - Refactor all backend initialization code paths and code style for consistency. v4: - Update to current trunk - Add missing param doc for mode in drm_output_choose_initial_mode - Rebase to account for code changes by 91880f1e to make vt switching configurable. Benoit Gschwind (2): x11: port the x11 backend to the new init api headless: port the headless backend to the new init api Bryce Harrington (9): drm: Spelling fix in comment Revert 'main: Remove unused function load_backend_new()' compositor: Drop unneeded create_output callback compositor: Version the backend configuration structures drm: Don't hang onto the backend config object post-backend_init drm: Move drm's output configuration into the drm backend drm: Pass gbm_format as enum rather than string in output config Enforce destruction of all backend config objects after initialization x11: Initialize all outputs at start Giulio Camuffo (1): drm: port the drm backend to the new init api Makefile.am | 9 ++ src/compositor-drm.c | 288 ++ src/compositor-drm.h | 86 ++ src/compositor-headless.c | 74 ++-- src/compositor-headless.h | 53 + src/compositor-x11.c | 158 + src/compositor-x11.h | 62 ++ src/compositor.h | 32 -- src/main.c| 230 +++- 9 files changed, 708 insertions(+), 284 deletions(-) create mode 100644 src/compositor-drm.h create mode 100644 src/compositor-headless.h create mode 100644 src/compositor-x11.h -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 10/12] drm: Pass gbm_format as enum rather than string in output config
Signed-off-by: Bryce Harrington
---
src/compositor-drm.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index d129adc..a896785 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -93,13 +93,13 @@ enum weston_drm_backend_output_mode {
struct weston_drm_backend_output_config {
struct weston_backend_output_config base;
- /** The pixel format to be used by the output. Valid values are:
-* - NULL - The format set at backend creation time will be used
-* - "xrgb"
-* - "rgb565"
-* - "xrgb2101010"
+ /** The pixel format to be used by the output. Supported values are:
+* - 0: The format set at backend creation time will be used
+* - GBM_FORMAT_XRGB
+* - GBM_FORMAT_RGB565
+* - GBM_FORMAT_XRGB2101010
*/
- char *gbm_format;
+ uint32_t gbm_format;
/** The seat to be used by the output. Set to NULL to use the
* default seat. */
@@ -2336,8 +2336,12 @@ drm_configure_output(struct weston_compositor *c,
s, name);
free(s);
- weston_config_section_get_string(section,
-"gbm-format", &config->gbm_format,
NULL);
+ weston_config_section_get_string(section, "gbm-format", &s, NULL);
+if (parse_gbm_format(s, GBM_FORMAT_XRGB, &config->gbm_format) < 0)
+ weston_log("Invalid gbm-format \"%s\" for output %s\n",
+ s, name);
+ free(s);
+
weston_config_section_get_string(section, "seat", &config->seat, "");
return mode;
}
@@ -2391,8 +2395,7 @@ create_output_for_connector(struct drm_backend *b,
mode = drm_configure_output(b->compositor, b->use_current_mode,
output->base.name, &config);
- if (parse_gbm_format(config.gbm_format, b->gbm_format,
&output->gbm_format) == -1)
- output->gbm_format = b->gbm_format;
+ output->gbm_format = config.gbm_format;
setup_output_seat_constraint(b, &output->base,
config.seat ? config.seat : "");
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 11/12] Enforce destruction of all backend config objects after initialization
Since the backend config struct versioning implies that there we expect
potential future descrepancy between main's definition of the config
object and the backend's, don't allow the backend to hang onto the
config object outside the initialization scope.
Signed-off-by: Bryce Harrington
Reviewed-by: Pekka Paalanen
v6:
- Put backend configs on stack instead of zalloc()
Signed-off-by: Bryce Harrington
---
src/main.c | 113 -
1 file changed, 51 insertions(+), 62 deletions(-)
diff --git a/src/main.c b/src/main.c
index dcd5ee6..9a8094f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -657,7 +657,20 @@ load_backend_old(struct weston_compositor *compositor,
const char *backend,
return backend_init(compositor, argc, argv, wc, NULL);
}
-/* Temporary function to be replaced by weston_compositor_load_backend(). */
+/** Main module call-point for backends.
+ *
+ * All backends should use this routine to access their init routine.
+ * Backends may subclass weston_backend_config to add their own
+ * configuration data, setting the major/minor version in config_base
+ * accordingly.
+ *
+ * The config_base object should be treated as temporary, and any data
+ * copied out of it by backend_init before returning. The load_backend_new
+ * callers may then free the config_base object.
+ *
+ * NOTE: This is a temporary function intended to eventually be replaced
+ * by weston_compositor_load_backend().
+ */
static int
load_backend_new(struct weston_compositor *compositor, const char *backend,
struct weston_backend_config *config_base)
@@ -678,38 +691,32 @@ static int
load_drm_backend(struct weston_compositor *c, const char *backend,
int *argc, char **argv, struct weston_config *wc)
{
- struct weston_drm_backend_config *config;
+ struct weston_drm_backend_config config = {{ 0, }};
struct weston_config_section *section;
int ret = 0;
- config = zalloc(sizeof (struct weston_drm_backend_config));
- if (!config)
- return -1;
-
const struct weston_option options[] = {
- { WESTON_OPTION_INTEGER, "connector", 0, &config->connector },
- { WESTON_OPTION_STRING, "seat", 0, &config->seat_id },
- { WESTON_OPTION_INTEGER, "tty", 0, &config->tty },
- { WESTON_OPTION_BOOLEAN, "current-mode", 0,
&config->use_current_mode },
- { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
+ { WESTON_OPTION_INTEGER, "connector", 0, &config.connector },
+ { WESTON_OPTION_STRING, "seat", 0, &config.seat_id },
+ { WESTON_OPTION_INTEGER, "tty", 0, &config.tty },
+ { WESTON_OPTION_BOOLEAN, "current-mode", 0,
&config.use_current_mode },
+ { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
};
parse_options(options, ARRAY_LENGTH(options), argc, argv);
section = weston_config_get_section(wc, "core", NULL, NULL);
weston_config_section_get_string(section,
-"gbm-format", &config->gbm_format,
+"gbm-format", &config.gbm_format,
NULL);
- config->base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
- config->base.struct_size = sizeof(struct weston_drm_backend_config);
+ config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
+ config.base.struct_size = sizeof(struct weston_drm_backend_config);
- ret = load_backend_new(c, backend,
- (struct weston_backend_config *)config);
+ ret = load_backend_new(c, backend, &config.base);
- free(config->gbm_format);
- free(config->seat_id);
- free(config);
+ free(config.gbm_format);
+ free(config.seat_id);
return ret;
}
@@ -718,38 +725,30 @@ static int
load_headless_backend(struct weston_compositor *c, char const * backend,
int *argc, char **argv, struct weston_config *wc)
{
- struct weston_headless_backend_config *config;
+ struct weston_headless_backend_config config = {{ 0, }};
int ret = 0;
const char *transform = "normal";
- config = zalloc(sizeof(struct weston_headless_backend_config));
- if (config == NULL)
- return -1;
-
- config->width = 1024;
- config->height = 640;
+ config.width = 1024;
+ config.height = 640;
const struct weston_option options[] = {
- { WESTON_OPTION_INTEGER, "width", 0, &config->width },
- { WESTON_OPTION_INTEGER, "height", 0, &config->height },
- { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
+ { WESTON_OPTION_INTEGER, "width", 0, &config.width },
+ { WESTON_OPTION_INTEGER, "height", 0, &config
[PATCH weston v6 12/12] x11: Initialize all outputs at start
We know ahead of time the maximum number of outputs we'll be
configuring, so just alloc that memory from the get-go, instead of
realloc'ing for each new output.
Suggested-by: Pekka Paalanen
Signed-off-by: Bryce Harrington
---
src/main.c | 37 -
1 file changed, 12 insertions(+), 25 deletions(-)
diff --git a/src/main.c b/src/main.c
index 9a8094f..99e5067 100644
--- a/src/main.c
+++ b/src/main.c
@@ -754,21 +754,6 @@ load_headless_backend(struct weston_compositor *c, char
const * backend,
}
static int
-weston_x11_backend_config_append_output_config(struct
weston_x11_backend_config *config,
- struct
weston_x11_backend_output_config *output_config) {
- struct weston_x11_backend_output_config *new_outputs;
-
- new_outputs = realloc(config->outputs, (config->num_outputs+1) *
- sizeof(struct weston_x11_backend_output_config));
- if (new_outputs == NULL)
- return -1;
-
- config->outputs = new_outputs;
- config->outputs[(config->num_outputs)++] = *output_config;
- return 0;
-}
-
-static int
load_x11_backend(struct weston_compositor *c, char const * backend,
int *argc, char **argv, struct weston_config *wc)
{
@@ -797,6 +782,15 @@ load_x11_backend(struct weston_compositor *c, char const *
backend,
parse_options(options, ARRAY_LENGTH(options), argc, argv);
+ if (option_count < 1) {
+ weston_log("No outputs configured\n");
+ return -1;
+ }
+ config.outputs = zalloc((option_count+1) *
+ sizeof(struct
weston_x11_backend_output_config));
+ if (config.outputs == NULL)
+ return -1;
+
section = NULL;
while (weston_config_next_section(wc, §ion, §ion_name)) {
struct weston_x11_backend_output_config current_output = { 0, };
@@ -842,11 +836,7 @@ load_x11_backend(struct weston_compositor *c, char const *
backend,
t, current_output.name);
free(t);
- if (weston_x11_backend_config_append_output_config(&config,
¤t_output) < 0) {
- ret = -1;
- goto out;
- }
-
+ config.outputs[(config.num_outputs)++] = current_output;
output_count++;
if (option_count && output_count >= option_count)
break;
@@ -861,13 +851,10 @@ load_x11_backend(struct weston_compositor *c, char const
* backend,
for (i = output_count; i < option_count; i++) {
if (asprintf(default_output.name, "screen%d", i) < 0) {
ret = -1;
- goto error;
- }
-
- if (weston_x11_backend_config_append_output_config(&config,
&default_output) < 0) {
- ret = -1;
goto out;
}
+
+ config.outputs[(config.num_outputs)++] = default_output;
}
config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v6 06/12] x11: port the x11 backend to the new init api
From: Benoit Gschwind
Use a "well" defined structure to configure x11-backend and move configuration
file parsing inside the weston compositor code.
Signed-off-by: Bryce Harrington
v6:
- Define version number in the header
- Don't use initial underscores in header guards
- Add stub config_init_to_defaults()
- Allocate config on stack
- Install compositor-x11.h and list it in x11-backend sources
- Fail if invalid width or height given for an output. It's the
caller's job to ensure the values are valid when parsing weston.ini
and the command line options.
- Preserve preference for height/width passed as options
v5:
- Update to current trunk
- Reformated for style consistency (e.g. spaces in "if(",
linebreaks, etc.)
- Move variable declarations to top of block
- Rename header guard for consistency with other headers
- Other misc. code style formatting fixes
- Adjust code style to better match drm backend config
- Version the config struct
- Dropped use of bzero in favor of zalloc
- Don't initialize vars already zalloc'd
- Dropped weston_x11_backend_load in favor of more general
load_backend_new routine
- Dropped weston_x11_backend_config_outputs_clear and just free
outputs in error handler for init routine
- Dropped some temp variables for output configuration
- Restore use of 'backend_init' as module entrypoint
- Rename 'ths' to 'config' for backend config structs
- Rename 'x11_options' to 'options' for consistency
- Rename other variables for consistency with other code
- Rename 'noutputs' to 'num_outputs'
Signed-off-by: Bryce Harrington
---
Makefile.am | 3 +
src/compositor-x11.c | 158 ++-
src/compositor-x11.h | 62
src/main.c | 147 ++-
4 files changed, 266 insertions(+), 104 deletions(-)
create mode 100644 src/compositor-x11.h
diff --git a/Makefile.am b/Makefile.am
index eeb40fb..dfa11ab 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -73,6 +73,7 @@ weston_SOURCES = \
src/compositor.c\
src/compositor.h\
src/compositor-drm.h\
+ src/compositor-x11.h\
src/input.c \
src/data-device.c \
src/screenshooter.c \
@@ -210,6 +211,7 @@ westoninclude_HEADERS = \
src/version.h \
src/compositor.h\
src/compositor-drm.h\
+ src/compositor-x11.h\
src/timeline-object.h \
shared/matrix.h \
shared/config-parser.h \
@@ -252,6 +254,7 @@ x11_backend_la_CFLAGS = \
$(AM_CFLAGS)
x11_backend_la_SOURCES = \
src/compositor-x11.c\
+ src/compositor-x11.h\
shared/helpers.h
endif
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 91a7c2e..629b5f3 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -50,21 +50,17 @@
#include
#include "compositor.h"
-#include "gl-renderer.h"
-#include "pixman-renderer.h"
+#include "compositor-x11.h"
#include "shared/config-parser.h"
#include "shared/helpers.h"
#include "shared/image-loader.h"
+#include "gl-renderer.h"
+#include "pixman-renderer.h"
#include "presentation-time-server-protocol.h"
#include "linux-dmabuf.h"
#define DEFAULT_AXIS_STEP_DISTANCE 10
-static int option_width;
-static int option_height;
-static int option_scale;
-static int option_count;
-
struct x11_backend {
struct weston_backendbase;
struct weston_compositor *compositor;
@@ -1566,25 +1562,16 @@ init_gl_renderer(struct x11_backend *b)
return ret;
}
+
static struct x11_backend *
x11_backend_create(struct weston_compositor *compositor,
- int fullscreen,
- int no_input,
- int use_pixman,
- int *argc, char *argv[],
- struct weston_config *config)
+ struct weston_x11_backend_config *config)
{
struct x11_backend *b;
struct x11_output *output;
struct wl_event_loop *loop;
- struct weston_config_section *section;
- int i, x = 0, output_count = 0;
- int width, height, scale, count;
- const char *section_name;
- char *name, *t, *mode;
- uint32_t transform;
-
- weston_log("initializing x11 backend\n");
+ int x = 0;
+ unsigned i;
b = zalloc(sizeof *b);
if (b == NULL)
@@ -1610,13 +1597,13 @@ x11_backend_create(struct weston_compositor *compos
[PATCH weston v6 07/12] headless: port the headless backend to the new init api
From: Benoit Gschwind
refactor configuration API of headless-backend
Signed-off-by: Bryce Harrington
Reviewed-by: Pekka Paalanen
v6:
- Define version number in the header
- Don't use leading underscores in header guards
- Add stub config_init_to_defaults()
- Allocate config on stack
- Drop unused display_name parameter
- Add error message when config is invalid
- Install compositor-headless.h and list it in headless-backend sources
v5:
- Update to current trunk
- Fixed typo 'struct weston_wayland_backend_config'
- Dropped unused variables
- Dropped weston_headless_backend_config_create() in favor of
directly zalloc'ing the object
- Dropped weston_headless_backend_load() in favor of the more
generalized load_backend_new().
- Dropped typedef from header
- Restored use of 'backend_init' entry point
- Backend_init() takes a base weston_backend_config object
- Renamed 'param' to 'config' in a few places for consistency
- Renamed 'headless_options' variable to 'options for consistency
- Version the base struct
- Free config on error
- Don't free config during backend_init normal operations
- Adjust header ordering
- Make header guard naming consistent with other headers
- Light reformatting for code style and consistency with other
backend config patches
Signed-off-by: Bryce Harrington
---
Makefile.am | 3 ++
src/compositor-headless.c | 74 +--
src/compositor-headless.h | 53 +
src/main.c| 45 ++--
4 files changed, 132 insertions(+), 43 deletions(-)
create mode 100644 src/compositor-headless.h
diff --git a/Makefile.am b/Makefile.am
index dfa11ab..b33d289 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -73,6 +73,7 @@ weston_SOURCES = \
src/compositor.c\
src/compositor.h\
src/compositor-drm.h\
+ src/compositor-headless.h \
src/compositor-x11.h\
src/input.c \
src/data-device.c \
@@ -211,6 +212,7 @@ westoninclude_HEADERS = \
src/version.h \
src/compositor.h\
src/compositor-drm.h\
+ src/compositor-headless.h \
src/compositor-x11.h\
src/timeline-object.h \
shared/matrix.h \
@@ -359,6 +361,7 @@ headless_backend_la_LIBADD = $(COMPOSITOR_LIBS) libshared.la
headless_backend_la_CFLAGS = $(COMPOSITOR_CFLAGS) $(AM_CFLAGS)
headless_backend_la_SOURCES = \
src/compositor-headless.c \
+ src/compositor-headless.h \
shared/helpers.h
endif
diff --git a/src/compositor-headless.c b/src/compositor-headless.c
index 16b5c39..ed6c48c 100644
--- a/src/compositor-headless.c
+++ b/src/compositor-headless.c
@@ -31,33 +31,29 @@
#include
#include
-#include "shared/helpers.h"
#include "compositor.h"
+#include "compositor-headless.h"
+#include "shared/helpers.h"
#include "pixman-renderer.h"
#include "presentation-time-server-protocol.h"
struct headless_backend {
struct weston_backend base;
struct weston_compositor *compositor;
+
struct weston_seat fake_seat;
bool use_pixman;
};
struct headless_output {
struct weston_output base;
+
struct weston_mode mode;
struct wl_event_source *finish_frame_timer;
uint32_t *image_buf;
pixman_image_t *image;
};
-struct headless_parameters {
- int width;
- int height;
- int use_pixman;
- uint32_t transform;
-};
-
static void
headless_output_start_repaint_loop(struct weston_output *output)
{
@@ -120,7 +116,7 @@ headless_output_destroy(struct weston_output *output_base)
static int
headless_backend_create_output(struct headless_backend *b,
- struct headless_parameters *param)
+ struct weston_headless_backend_config *config)
{
struct weston_compositor *c = b->compositor;
struct headless_output *output;
@@ -132,15 +128,15 @@ headless_backend_create_output(struct headless_backend *b,
output->mode.flags =
WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
- output->mode.width = param->width;
- output->mode.height = param->height;
+ output->mode.width = config->width;
+ output->mode.height = config->height;
output->mode.refresh = 6;
wl_list_init(&output->base.mode_list);
wl_list_insert(&output->base.mode_list, &output->mode.link);
output->base.current_mode = &output->mode;
- weston_out
[PATCH weston v6 04/12] compositor: Version the backend configuration structures
With this struct versioning, it is possible to add new options without
breaking the ABI, as long as all additions are made to the end of a
struct and nothing existing is modified or removed. When things are
added, the structure's size will increase, and we'll use this size as
our minor version number. If existing things need to be changed, then
the major version, struct_version, is incremented to indicate the ABI
break.
From our call sites in main these major and minor version will be
recorded as struct_version and struct_size. Each backend will then
verify these against its own assumptions. So long as the backend's
struct is equal or larger than what was passed in and the major versions
are equal, we're good; but if it is larger, then this is a fatal error.
Signed-off-by: Bryce Harrington
Reviewed-by: Pekka Paalanen
v6:
- Document refs for alternatives/assumptions for backend configs
v5:
- Move the header changes to a pre-requisite patch from the drm backend
Signed-off-by: Bryce Harrington
---
src/compositor.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/src/compositor.h b/src/compositor.h
index 5ca497c..a329dbe 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -684,8 +684,30 @@ struct weston_backend_output_config {
* passed to the backend's init entry point. The backend will
* likely want to subclass this in order to handle backend specific
* data.
+ *
+ * NOTE: Alternate designs were proposed (Feb 2016) for using opaque
+ * structures and for section+key/value getter/setters. The rationale
+ * for selecting the transparent structure design is based on several
+ * assumptions which may require re-evaluating the design choice if they
+ * fail to hold.
*/
struct weston_backend_config {
+ /** Major version for the backend-specific config struct
+*
+* This version must match exactly what the backend expects, otherwise
+* the struct is incompatible.
+*/
+ uint32_t struct_version;
+
+ /** Minor version of the backend-specific config struct
+*
+* This must be set to sizeof(struct backend-specific config).
+* If the value here is smaller than what the backend expects, the
+* extra config members will assume their default values.
+*
+* A value greater than what the backend expects is incompatible.
+*/
+ size_t struct_size;
};
struct weston_backend {
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v6 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests
On Fri, Apr 15, 2016 at 03:46:43AM -0400, Olivier Fourdan wrote: > > Hi, > > - Original Message - > > On Thu, Apr 14, 2016 at 1:56 AM, Jonas Ådahl wrote: > > > > > > > > This makes it sound like the compositor will have to obay the limits. > > > Did we end up with requiring that a correctly working compositor never > > > tries to configure a size that is not within the set bounds? I think it > > > should be cleared out anyhow, what the client can rely upon here anyhow. > > > > > > > No the compositor is not required to obey the limits! This whole discussion > > was based on examples of tiled window management and complaints about the > > calculator being ugly due to being scaled, compared to X11 where the tiled > > window manager requested sizes outside the size range, and the client > > obeyed it. > [...] > > If the compositor cannot request a larger size then all these patches are > > entirely pointless!!! > > Seems everyone has a different idea and a consensus might be hardly > achievable here. > > Thing is, the client still have the option of not ack'ing the configure > event, so the final word is (and remains) with the client, that's it. > > Considering this, not sure we need to rephrase the description once again. Given that different people have a different idea of how this is supposed to work, we need to make it very clear of what the correct interpretation is. Leaving it as vague as it right now is doesn't seem like a good idea. Either it is purely a polite suggestion, or it is an actual limit, and to me it the current description doesn't communicate this very clearly. Jonas > > Cheers, > Olivier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v5 03/11] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 02:32:04PM +0200, Benoit Gschwind wrote: > > + /** The seat to be used by the output. Set to NULL to use the > > +* default seat. */ > > + char *seat; > > + /** The modeline to be used by the output. Refer to the documentation > > +* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */ > > + char *modeline; > > Maybe enum with drmModeModeInfo* is better API than free string API? I think you're probably right that weston_drm_backend_output_config could hold a pointer to the enum rather than the string. I've moved weston_drm_backend_output_config to be a private struct in compositor-drm.c so this should be doable. But I think I'm going to opt to leave this change to Giulio as follow-up work, as I think it needs deeper thought than I'm going to be able to give it in this patchset. There was also some discussion about whether WESTON_DRM_BACKEND_OUTPUT_PREFERRED should be renamed, in which case it would make sense to do the change in conjunction with that. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/5] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 12:01:01PM +0300, Pekka Paalanen wrote:
> On Thu, 14 Apr 2016 19:09:34 +0300
> Giulio Camuffo wrote:
>
> > 2016-04-13 14:30 GMT+03:00 Pekka Paalanen :
> > > On Tue, 12 Apr 2016 21:34:28 -0700
> > > Bryce Harrington wrote:
> > >
> > >> On Wed, Apr 06, 2016 at 11:37:57AM +0300, Pekka Paalanen wrote:
> > >> > On Wed, 9 Mar 2016 16:49:29 -0800
> > >> > Bryce Harrington wrote:
> > >> >
> > >> > > From: Giulio Camuffo
> > >> > >
> > >> > > Signed-off-by: Bryce Harrington
> > >> > > Reviewed-by: Quentin Glidic
> > >> > > Acked-by: Pekka Paalanen
> > >> > > ---
> > >> > > v4: Update to current trunk
> > >> > > - Add missing param doc for mode in
> > >> > > drm_output_choose_initial_mode
> > >> > > - Rebase to account for code changes by 91880f1e to make vt
> > >> > > switching configurable.
> > >> > >
> > >> > > Makefile.am | 3 +
> > >> > > src/compositor-drm.c | 196
> > >> > > ++-
> > >> > > src/compositor.h | 2 -
> > >> > > src/main.c | 94 +++-
> > >> > > 4 files changed, 165 insertions(+), 130 deletions(-)
> > >> >
> > >> > Hi Giulio and Bryce,
> > >> >
> > >> > I'm sorry it has taken so long for me to come back to this.
> > >
> > >> > > +static int
> > >> > > +load_drm_backend(struct weston_compositor *c, const char *backend,
> > >> > > + int *argc, char **argv, struct weston_config *wc)
> > >> > > +{
> > >> > > + struct drm_config *config;
> > >> > > + struct weston_config_section *section;
> > >> > > + int ret = 0;
> > >> > > +
> > >> > > + config = zalloc(sizeof *config);
> > >> > > + if (!config)
> > >> > > + return -1;
> > >> >
> > >> > This function will be affected by the struct versioning too.
> > >>
> > >> The subsequent patch adds the versioning, although later in the routine
> > >> than here.
> > >>
> > >> > > + const struct weston_option options[] = {
> > >> > > + { WESTON_OPTION_INTEGER, "connector", 0,
> > >> > > &config->base.connector },
> > >> > > + { WESTON_OPTION_STRING, "seat", 0, &config->base.seat_id },
> > >> > > + { WESTON_OPTION_INTEGER, "tty", 0, &config->base.tty },
> > >> > > + { WESTON_OPTION_BOOLEAN, "current-mode", 0,
> > >> > > + &config->use_current_mode },
> > >> > > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0,
> > >> > > &config->base.use_pixman },
> > >> > > + };
> > >> >
> > >> > Mixed declarations and code.
> > >>
> > >> Hmm, I'm not sure best how to address this. Options is being
> > >> initialized with pointers that are created by the zalloc, so I don't
> > >> think I can just separate the declaration from the initialization. Does
> > >> options need to be changed to be dynamically allocated as well?
> > >
> > > Hi,
> > >
> > > I suppose the simplest solution is to make another function, define
> > > 'options[]' in it, and get 'config' as an argument. Then it can call
> > > the parser and return. Something like ...fill_from_command_line(struct
> > > drm_config *config, argc, argv...).
> > >
> > > It's fine to keep your changes as separate patches as long as every
> > > patch is bisectable.
> > >
> > > I'm going through the new series right now, and noticed these emails
> > > from you just now.
> >
> > I think this is one of the cases where strict style compliance goes in
> > the way of better code. Adding a function just adds an indirect level
> > but gives nothing in return, because it would really be just a wapper
> > around the options[] variable. I don't see it much different to a "int
> > value_1() { return 1; }".
> > But more importantly imho, it requires another revision.
>
> No, it would be much more. The function would essentially be "fill in
> this config struct from these argc,argv". That color would actually
> please my eye more, as that is a logical single step to execute from a
> load_*() function, rather than stuffing it all inline in load_*().
>
> But, I'm not the one building the shed here. If I really care, I can
> fix it after landing this stuff, since it doesn't seem to break the
> build.
One wrinkle I'm spotting as I try to implement this is that the headless
and x11 backends have options that are parsed but not stored in the
config structure. Maybe I can shove them in anyway though.
> Thanks,
> pq
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2
>
> iQIVAwUBVxCtzSNf5bQRqqqnAQiYHhAAmj+UCgMJGEj0vUglcKIx0FAsYQylqNm5
> zn0cYwGsi/mkbhCy5A6TNwTM5y21DGW7+/wcSwnlBZLH/7PDjX57ppwy+s7jcW5m
> 9dLi/qpOlepfQ8S0BF1epFlQAyxBqa1JLvfI3O3ta91ku5VBCCEnKLNyqwCGE8z3
> COUudldrCP6Ysb9XY2TXLhlwDZxDu1uVeEXS2G1UxEj/ealB5RXypzuPKTSYVl77
> WsDhRgP1GMo4g1XL7B5cvBkvrrzDkj31u+M+pffIcvBJjnpL7ja7QSkerylXyaP7
> 1z4XIqM0vsyzxEnh7ZhNLIHcP1kOhunjJJi9/ti36DRwcL/flMIQoYElQU1MvBmw
> Jv9tm2XlMjTIPMKgtdbSaz6fu6sJq0oEUQXYhpTMhaNx0H8Cg8uBpwQpaWsKP1A1
> QLM86aCpQsiNDPsvgTStX5M2hoJ0+MJbg/hpHy1ylhigaC3B8sx6nXX9JcGAEUBJ
>
Re: [PATCH 2/5] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 12:01:01PM +0300, Pekka Paalanen wrote:
> On Thu, 14 Apr 2016 19:09:34 +0300
> Giulio Camuffo wrote:
>
> > 2016-04-13 14:30 GMT+03:00 Pekka Paalanen :
> > > On Tue, 12 Apr 2016 21:34:28 -0700
> > > Bryce Harrington wrote:
> > >
> > >> On Wed, Apr 06, 2016 at 11:37:57AM +0300, Pekka Paalanen wrote:
> > >> > On Wed, 9 Mar 2016 16:49:29 -0800
> > >> > Bryce Harrington wrote:
> > >> >
> > >> > > From: Giulio Camuffo
> > >> > >
> > >> > > Signed-off-by: Bryce Harrington
> > >> > > Reviewed-by: Quentin Glidic
> > >> > > Acked-by: Pekka Paalanen
> > >> > > ---
> > >> > > v4: Update to current trunk
> > >> > > - Add missing param doc for mode in
> > >> > > drm_output_choose_initial_mode
> > >> > > - Rebase to account for code changes by 91880f1e to make vt
> > >> > > switching configurable.
> > >> > >
> > >> > > Makefile.am | 3 +
> > >> > > src/compositor-drm.c | 196
> > >> > > ++-
> > >> > > src/compositor.h | 2 -
> > >> > > src/main.c | 94 +++-
> > >> > > 4 files changed, 165 insertions(+), 130 deletions(-)
> > >> >
> > >> > Hi Giulio and Bryce,
> > >> >
> > >> > I'm sorry it has taken so long for me to come back to this.
> > >
> > >> > > +static int
> > >> > > +load_drm_backend(struct weston_compositor *c, const char *backend,
> > >> > > + int *argc, char **argv, struct weston_config *wc)
> > >> > > +{
> > >> > > + struct drm_config *config;
> > >> > > + struct weston_config_section *section;
> > >> > > + int ret = 0;
> > >> > > +
> > >> > > + config = zalloc(sizeof *config);
> > >> > > + if (!config)
> > >> > > + return -1;
> > >> >
> > >> > This function will be affected by the struct versioning too.
> > >>
> > >> The subsequent patch adds the versioning, although later in the routine
> > >> than here.
> > >>
> > >> > > + const struct weston_option options[] = {
> > >> > > + { WESTON_OPTION_INTEGER, "connector", 0,
> > >> > > &config->base.connector },
> > >> > > + { WESTON_OPTION_STRING, "seat", 0, &config->base.seat_id },
> > >> > > + { WESTON_OPTION_INTEGER, "tty", 0, &config->base.tty },
> > >> > > + { WESTON_OPTION_BOOLEAN, "current-mode", 0,
> > >> > > + &config->use_current_mode },
> > >> > > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0,
> > >> > > &config->base.use_pixman },
> > >> > > + };
> > >> >
> > >> > Mixed declarations and code.
> > >>
> > >> Hmm, I'm not sure best how to address this. Options is being
> > >> initialized with pointers that are created by the zalloc, so I don't
> > >> think I can just separate the declaration from the initialization. Does
> > >> options need to be changed to be dynamically allocated as well?
> > >
> > > Hi,
> > >
> > > I suppose the simplest solution is to make another function, define
> > > 'options[]' in it, and get 'config' as an argument. Then it can call
> > > the parser and return. Something like ...fill_from_command_line(struct
> > > drm_config *config, argc, argv...).
> > >
> > > It's fine to keep your changes as separate patches as long as every
> > > patch is bisectable.
> > >
> > > I'm going through the new series right now, and noticed these emails
> > > from you just now.
> >
> > I think this is one of the cases where strict style compliance goes in
> > the way of better code. Adding a function just adds an indirect level
> > but gives nothing in return, because it would really be just a wapper
> > around the options[] variable. I don't see it much different to a "int
> > value_1() { return 1; }".
> > But more importantly imho, it requires another revision.
>
> No, it would be much more. The function would essentially be "fill in
> this config struct from these argc,argv". That color would actually
> please my eye more, as that is a logical single step to execute from a
> load_*() function, rather than stuffing it all inline in load_*().
>
> But, I'm not the one building the shed here. If I really care, I can
> fix it after landing this stuff, since it doesn't seem to break the
> build.
I'm going to see how it looks broken out as a separate function as pq
suggests. All this code only gets executed once and there's no loops or
anything, so the extra level of indirection isn't really a performance
issue. But since it deals with command options it's likely to get
tinkered with by a large variety of people in the future, so clarity and
maintainability seem like the main concerns to worry about. From that
viewpoint I'm not sure which of "keeping everything together in one
place" or "having things nicely organized into short functions" becomes
the stronger argument.
The other goal is keeping things stylistically consistent with the
other backends; if we break the options out into their own function
here, then it should be done that same way in all the backends.
Bryce
___
Re: [PATCH weston v5 00/11] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 03:17:15PM +0200, Benoit Gschwind wrote: > Hello Bryce, > > The patches set was tested above [1] > > I tested drm-bakcend and x11-backend successfully with the full patchset. > > Here are the resume of my comments and review. I send separate e-mail > when contextual comments are required. > > PATCH 01/11: > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 02/11: > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 03/11 > > Already commented in another e-mail > > PATCH 04/11 > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 05/11 > > Do what is expected. > > Reviewed-by: Benoit Gschwind > > PATCH 06/11 > > Agree with Pekka comments, I'm available to fix issues if needed. > > PATCH 07/11 > > Agree with Pekka comments, I'm available to fix issues if needed. Thanks for the offer; I think since there's other changes to be made I'll try to tackle them. But, there is one thing you could help with - there is another patch you posted some time back for the wayland backend, which needs to be updated to conform to the design and style of these patches. If you could update it, then we could get it landed too. Bryce > PATCH 08/11 > > The re-factor look functional. Other decision tie to the community :) > > PATCH 09/11 > > This patch look to address some concern I raised in the patch 03/11, but > seems to need more discussion. > > PATCH 10/11 > > The patch look to address issue of PATCH 07/11 that Pekka raised about > freeing config variable. > > PATCH 11/11 > > Look fine. > > Reviewed-by: Benoit Gschwind > > > > [1] git node: 94cb06a208130b0ee16553a2cd513e5e7d67f368 > > > > Le 13/04/2016 12:25, Bryce Harrington a écrit : > > In following up on my earlier update of Giulio's drm backend config > > patch, I've taken the liberty to try and also integrate a couple of > > Benoit's other backend configuration patches into this patchset. > > > > Giulio and Benoit took different approaches in their implementations. > > I've attempted to reconcile them so that they are stylistically > > consistent, along with incorporating pq's request to have the root > > config structure incorporate versioning information. I went through all > > the review comments against each of the backend config patches and > > incorporated the suggestions people made. The resultant changes have > > ended up a bit more extensive than I had expected, and I apologize for > > any toe stepping I'm doing but my hope is to move the collective work > > closer to being landable. > > > > --- > > todo: > > - Use backend-specific header #defines for struct_version values > > v5: > > - Add missing compositor-drm.h > > - Integrate wayland, x11, and headless backend patches > > - Implement struct versioning for wayland, x11, and headless backend > > patches in the weston_backend_config structure, as requested by pq > > - Drop bzero usage as suggested in review by Jan Engelhardt > > - Don't change 'backend_init' entry point function name, as suggested > > in review by Giulio > > - Prefer use of load_backend_new() for actual module loading, as > > suggested in Giulio's review of the x11 backend patch > > - Refactor all backend initialization code paths and code style for > > consistency. > > - Squashed drm config struct versioning with drm patch; remainder moved > > to a prerequisite patch. > > v4: > > - Update to current trunk > > - Add missing param doc for mode in drm_output_choose_initial_mode > > - Rebase to account for code changes by 91880f1e to make vt > > switching configurable. > > > > > > > > Benoit Gschwind (1): > > x11: port the x11 backend to the new init api > > > > Bryce Harrington (9): > > Revert "main: Remove unused function load_backend_new()" > > compositor: Version the backend configuration structures > > drm: Fix gcc warning about missing braces. > > compositor: Document refs for alternatives/assumptions for backend > > configs > > headless: port the headless backend to the new init api > > drm: Code and comments reformatting for consistency with other backend > > configs > > drm: Don't hang onto the backend config object post-backend_init > > Enforce destruction of all backend config objects after initialization > > drm: Drop use of drm_config config wrapper > > > > Giulio Camuffo (1): > > drm: port the drm backend to the new init api > > > > Makefile.am | 5 + > > src/compositor-drm.c | 216 ++-- > > src/compositor-drm.h | 125 +++ > > src/compositor-headless.c | 69 +-- > > src/compositor-headless.h | 51 > > src/compositor-x11.c | 157 > > src/compositor-x11.h | 60 + > > src/compositor.h | 24 +++- > > src/main.c| 304 > > +- > > 9 files ch
Re: [PATCH weston v5 03/11] drm: port the drm backend to the new init api
On Fri, Apr 15, 2016 at 02:32:04PM +0200, Benoit Gschwind wrote:
> Hello Bryce and Giulio,
>
> Thanks for your contribution :), here is my comments.
>
> Le 13/04/2016 12:25, Bryce Harrington a écrit :
> > From: Giulio Camuffo
> >
> > Signed-off-by: Bryce Harrington
> > Reviewed-by: Quentin Glidic
> > Acked-by: Pekka Paalanen
> >
> > ---
> > v5:
> > - Update to reflect format rename to gdb_format
> > - Initialize width/height (suggested by pq)
> > - squash drm structure versioning (suggested by pq)
> >
> > Makefile.am | 3 +
> > src/compositor-drm.c | 197
> > ---
> > src/compositor-drm.h | 111 +
> > src/compositor.h | 2 -
> > src/main.c | 96 -
> > 5 files changed, 283 insertions(+), 126 deletions(-)
> > create mode 100644 src/compositor-drm.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index d1644ac..f4cff4c 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -72,6 +72,7 @@ weston_SOURCES = \
> > src/log.c \
> > src/compositor.c\
> > src/compositor.h\
> > + src/compositor-drm.h\
> > src/input.c \
> > src/data-device.c \
> > src/screenshooter.c \
> > @@ -207,6 +208,7 @@ westonincludedir = $(includedir)/weston
> > westoninclude_HEADERS =\
> > src/version.h \
> > src/compositor.h\
> > + src/compositor-drm.h\
> > src/timeline-object.h \
> > shared/matrix.h \
> > shared/config-parser.h \
> > @@ -276,6 +278,7 @@ drm_backend_la_CFLAGS = \
> > $(AM_CFLAGS)
> > drm_backend_la_SOURCES = \
> > src/compositor-drm.c\
> > + src/compositor-drm.h\
> > $(INPUT_BACKEND_SOURCES)\
> > shared/helpers.h\
> > shared/timespec-util.h \
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 119b6c5..508b7e8 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -50,6 +50,7 @@
> > #include "shared/timespec-util.h"
> > #include "libbacklight.h"
> > #include "compositor.h"
> > +#include "compositor-drm.h"
> > #include "gl-renderer.h"
> > #include "pixman-renderer.h"
> > #include "libinput-seat.h"
> > @@ -74,17 +75,6 @@
> > #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> > #endif
> >
> > -static int option_current_mode = 0;
> > -
> > -enum output_config {
> > - OUTPUT_CONFIG_INVALID = 0,
> > - OUTPUT_CONFIG_OFF,
> > - OUTPUT_CONFIG_PREFERRED,
> > - OUTPUT_CONFIG_CURRENT,
> > - OUTPUT_CONFIG_MODE,
> > - OUTPUT_CONFIG_MODELINE
> > -};
> > -
> > struct drm_backend {
> > struct weston_backend base;
> > struct weston_compositor *compositor;
> > @@ -130,6 +120,8 @@ struct drm_backend {
> >
> > int32_t cursor_width;
> > int32_t cursor_height;
> > +
> > + struct weston_drm_backend_config *config;
> > };
> >
> > struct drm_mode {
> > @@ -220,13 +212,6 @@ struct drm_sprite {
> > uint32_t formats[];
> > };
> >
> > -struct drm_parameters {
> > - int connector;
> > - int tty;
> > - int use_pixman;
> > - const char *seat_id;
> > -};
> > -
> > static struct gl_renderer_interface *gl_renderer;
> >
> > static const char default_seat[] = "seat0";
> > @@ -2151,31 +2136,23 @@ setup_output_seat_constraint(struct drm_backend *b,
> > }
> >
> > static int
> > -get_gbm_format_from_section(struct weston_config_section *section,
> > - uint32_t default_value,
> > - uint32_t *format)
> > +parse_gbm_format(const char *s, uint32_t default_value, uint32_t
> > *gbm_format)
> > {
> > - char *s;
> > int ret = 0;
> >
> > - weston_config_section_get_string(section,
> > -"gbm-format", &s, NULL);
> > -
> > if (s == NULL)
> > - *format = default_value;
> > + *gbm_format = default_value;
> > else if (strcmp(s, "xrgb") == 0)
> > - *format = GBM_FORMAT_XRGB;
> > + *gbm_format = GBM_FORMAT_XRGB;
> > else if (strcmp(s, "rgb565") == 0)
> > - *format = GBM_FORMAT_RGB565;
> > + *gbm_format = GBM_FORMAT_RGB565;
> > else if (strcmp(s, "xrgb2101010") == 0)
> > - *format = GBM_FORMAT_XRGB2101010;
> > + *gbm_format = GBM_FORMAT_XRGB2101010;
> > else {
> > weston_log("fatal: unrecognized pixel format: %s\n", s);
> > ret = -1;
> > }
> >
> > - free(s);
> > -
> > return ret;
> > }
> >
> > @@ -2194,
Re: [PATCH] option-parser: Handle short double-arg options
On Fri, Apr 15, 2016 at 11:55:57AM +0300, Pekka Paalanen wrote: > On Thu, 14 Apr 2016 11:10:57 -0700 > Bryce Harrington wrote: > > > On Thu, Apr 14, 2016 at 03:16:07PM +0300, Pekka Paalanen wrote: > > > On Sat, 13 Feb 2016 23:56:38 +0100 > > > Benoit Gschwind wrote: > > > > > > > Hello Bryce, > > > > > > > > It seems the corner case '-f42xxx 2938475' doesn't work as expected > > > > with > > > > 'f' short option as integer: > > > > > > > > 1. one dash then call short_option > > > > 2. in short_option will check arg[2] and call handle_option > > > > 3. in handle_option will call strtol, where *value is '4' and *p is 'x' > > > > thus *value && !*p is 0 > > > > 4. return from handle_option with 0 > > > > 5. return from short_option with 0 > > > > 6. call short_option_with_arg with arg = "-f42xxx" and param = "2938475" > > > > 7. pass through all test and call handle_option > > > > 8. give 2938475 to the option > > > > > > > > The expected behavior is an error. > > > > > > > > Should I use Reviewed-by ? > > > > > > No, you only give Reviewed-by when you think everything is good. You > > > are pointing out a problem instead. > > > > > > Looks like this patch landed. Isn't there something to fix? > > > > The option handling code is relatively concise, which is good but the > > consequence is that there's a heap of corner cases like the above. > > > > Personally I don't think it's worth going overboard in trying to solve > > every potential case, just ones that are likely to come up in use, > > because if we do want a really robust option parsing system we probably > > should be looking at one of the several widely used option handling > > libraries like popt or whatnot. > > > > Hi Bryce, > > in that case, shouldn't we just revert this patch? This solved a problem that came up in actual use, while I was testing the idle inhibition functionality. I did try to keep the patch as minimal as possible though. > > > I think we might have tests for the option parser. Would be good to add > > > this particular corner-case as a test there. If we don't have tests, > > > would be nice to have some. > > > > Of course I'm a strong +1 for more tests, but again I think if we care > > that much about making the option handling solid, we really ought first > > do some due diligence looking at existing solutions. This isn't an area > > where we're value-adding much... > > > > Either way we go, I'd be happy to integrate a replacement lib or improve > > our existing solution, although I have some other matters that would be > > higher priority in the near term. > > Personally I've been ok with what it was. I have forgotten why it > didn't originally just wrap getopt_long (a GNUism? too much effort?). Or portability maybe? getopt_long has a few quirks/limitations itself (for Inkscape we switched to popt). > But you're right, it's a really unimportant thing, so I won't push it > in any direction myself. It just looked like you ignored a review > comment pointing out a new problem. Sorry. We did actually exchange emails about it off-list. > Changing the command line argument syntax might even be too disruptive > nowadays. Possibly, yeah, although some packages are pretty flexible and I would expect could replicate our syntax style adequately enough. I agree that avoiding disruption would have to be a requirement in making any choice. I suppose one thing to consider is that with the libweston backend configuration support going in, there might be more pressure for more featureful command line option functionality. But that pressure will be pretty obvious if it comes. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-build-tools] wl_install_deps: Add missing dependencies
On Thu, Apr 14, 2016 at 05:48:12PM -0500, Yong Bakos wrote:
> From: Yong Bakos
>
> Building on Ubuntu 15.10 x64 fails due to missing packages:
> libtool, libxml2-dev, libpng-dev, libglib2.0-dev, libgcrypt20-dev,
> x11proto-scrnsaver-dev, libxfont-dev, and libedit-dev.
>
> After these changes, wl_build succeeds on Ubuntu 15.10 x64.
>
> Signed-off-by: Yong Bakos
Looks good, pushed:
452f1d9..36f2bd1 master -> master
> ---
> wl_install_deps | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/wl_install_deps b/wl_install_deps
> index 2c852d8..ea4b69b 100755
> --- a/wl_install_deps
> +++ b/wl_install_deps
> @@ -3,7 +3,7 @@
> . $HOME/.config/wayland-build-tools/wl_defines.sh
>
> # generic build dependencies for ubuntu
> -sudo apt-get -y install autoconf automake bison debhelper dpkg-dev flex
> +sudo apt-get -y install autoconf automake bison debhelper dpkg-dev flex
> libtool
> sudo apt-get -y install pkg-config quilt python-libxml2
>
> # wayland/weston specific stuff
> @@ -20,6 +20,13 @@ sudo apt-get -y install libudev-dev
> sudo apt-get -y install libgudev-1.0-dev
> sudo apt-get -y install llvm-dev
> sudo apt-get -y install python-mako
> +sudo apt-get -y install libxml2-dev
> +sudo apt-get -y install libpng-dev
> +sudo apt-get -y install libglib2.0-dev
> +sudo apt-get -y install libgcrypt20-dev
> +sudo apt-get -y install x11proto-scrnsaver-dev
> +sudo apt-get -y install libxfont-dev
> +sudo apt-get -y install libedit-dev
>
> # xwayland specific stuff
> if [ ${INCLUDE_XWAYLAND} ]; then
> --
> 2.7.2
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v3] protocol: Extend wl_touch with touchpoint shape and orientation
- Fixed the rebase error, some typos and formatting as suggested by Yong. - I did not notice that the file is mixing tabs and spaces, so I fixed that as well. - Split the event into orientation and shape as suggested by Peter. On Fri, Apr 15, 2016 at 8:01 AM, Dennis Kempin wrote: > This CL updates the wl_touch interface with a shape and > orientation event. > The shape/orientation of a touch point is not relevant for most UI > applications, but allows a better experience in some cases > such as drawing apps. > > The events are used by the compositor to inform the client > about changes in the shape and orientation of a touchpoint, which is > approximated by an ellipse and it's angle to the y-axis. > > The event is optional and only sent when compositor and the > touch device support this type of information. The client is > responsible for making a reasonable assumption about the > touch shape if no shape is reported. > > Signed-off-by: Dennis Kempin > --- > protocol/wayland.xml | 64 > > 1 file changed, 59 insertions(+), 5 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index 12a6efd..b8a9835 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -1656,7 +1656,7 @@ > > > > - > + > >A seat is a group of keyboards, pointer and touch devices. This >object is published as a global during start up, or when such a > @@ -1765,7 +1765,7 @@ > > > > - > + > >The wl_pointer interface represents one or more input devices, >such as mice, which control the pointer location and pointer_focus > @@ -2078,7 +2078,7 @@ > > > > - > + > >The wl_keyboard interface represents one or more keyboards >associated with a seat. > @@ -2192,7 +2192,7 @@ > > > > - > + > >The wl_touch interface represents a touchscreen >associated with a seat. > @@ -2242,7 +2242,12 @@ > > > > - Indicates the end of a contact point list. > + Indicates the end of a contact point list. The wayland protocol requires > + touch point updates to be sent sequentially, however all events within a > + frame should be considered one hardware event. A wl_touch.frame terminates > + at least one event but otherwise no guarantee is provided about the set of > + events within a frame. A client must assume that any state not updated in a > + frame is unchanged from the previously known state. > > > > @@ -2262,6 +2267,55 @@ > > > > + > + > + > + > + > + Sent when a touchpoint has changed its shape. If the touch position > + or orientation changed at the same time, the wl_touch.motion, > + wl_touch.orientation and wl_touch.shape are sent within the same > + wl_touch.frame. Otherwise, only a wl_touch.shape is sent within this > + wl_touch.frame. The protocol does not guarantee specific ordering of > + wl_touch.orientation, wl_touch.shape and wl_touch.motion events. > + > + A touchpoint shape is approximated by an ellipse through the major and minor > + axis length. The major axis length describes the longest diameter of the > + ellipse, while the minor axis length describes the shortest diameter. > + Both are specified in surface coordinates. The center of the ellipse is > + always at the touchpoint location as reported by wl_touch.down or > + wl_touch.move. > + > + This event is only sent by the compositor if the touch device supports shape > + reports. The client has to make reasonable assumptions about the shape if > + it did not receive this event. > + > + > + > + > + > + > + > + > + Sent when a touchpoint has changed its orientation. If the touch position > + or shape changed at the same time, the wl_touch.motion, wl_touch.orientation > + and wl_touch.shape are sent within the same wl_touch.frame. > + Otherwise, only a wl_touch.orientation is sent within this wl_touch.frame. > + The protocol does not guarantee specific ordering of wl_touch.orientation, > + wl_touch.shape and wl_touch.motion events. > + > + The orientation describes the clockwise angle of touchpoints major axis to > + the surface y-axis and is normalized to the -180 to +180 degrees range. > + The granuality of orientation depends on the touch device, some devices only > + support binary rotation values between 0 and 90 degrees. > + > + This event is only sent by the compositor if the touch device supports > + orientation reports. > + > + > + > + > + > > > > -- > 2.8.0.rc3.226.g39d4020 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v3] protocol: Extend wl_touch with touchpoint shape and orientation
This CL updates the wl_touch interface with a shape and orientation event. The shape/orientation of a touch point is not relevant for most UI applications, but allows a better experience in some cases such as drawing apps. The events are used by the compositor to inform the client about changes in the shape and orientation of a touchpoint, which is approximated by an ellipse and it's angle to the y-axis. The event is optional and only sent when compositor and the touch device support this type of information. The client is responsible for making a reasonable assumption about the touch shape if no shape is reported. Signed-off-by: Dennis Kempin --- protocol/wayland.xml | 64 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 12a6efd..b8a9835 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1656,7 +1656,7 @@ - + A seat is a group of keyboards, pointer and touch devices. This object is published as a global during start up, or when such a @@ -1765,7 +1765,7 @@ - + The wl_pointer interface represents one or more input devices, such as mice, which control the pointer location and pointer_focus @@ -2078,7 +2078,7 @@ - + The wl_keyboard interface represents one or more keyboards associated with a seat. @@ -2192,7 +2192,7 @@ - + The wl_touch interface represents a touchscreen associated with a seat. @@ -2242,7 +2242,12 @@ - Indicates the end of a contact point list. + Indicates the end of a contact point list. The wayland protocol requires + touch point updates to be sent sequentially, however all events within a + frame should be considered one hardware event. A wl_touch.frame terminates + at least one event but otherwise no guarantee is provided about the set of + events within a frame. A client must assume that any state not updated in a + frame is unchanged from the previously known state. @@ -2262,6 +2267,55 @@ + + + + + + Sent when a touchpoint has changed its shape. If the touch position + or orientation changed at the same time, the wl_touch.motion, + wl_touch.orientation and wl_touch.shape are sent within the same + wl_touch.frame. Otherwise, only a wl_touch.shape is sent within this + wl_touch.frame. The protocol does not guarantee specific ordering of + wl_touch.orientation, wl_touch.shape and wl_touch.motion events. + + A touchpoint shape is approximated by an ellipse through the major and minor + axis length. The major axis length describes the longest diameter of the + ellipse, while the minor axis length describes the shortest diameter. + Both are specified in surface coordinates. The center of the ellipse is + always at the touchpoint location as reported by wl_touch.down or + wl_touch.move. + + This event is only sent by the compositor if the touch device supports shape + reports. The client has to make reasonable assumptions about the shape if + it did not receive this event. + + + + + + + + + Sent when a touchpoint has changed its orientation. If the touch position + or shape changed at the same time, the wl_touch.motion, wl_touch.orientation + and wl_touch.shape are sent within the same wl_touch.frame. + Otherwise, only a wl_touch.orientation is sent within this wl_touch.frame. + The protocol does not guarantee specific ordering of wl_touch.orientation, + wl_touch.shape and wl_touch.motion events. + + The orientation describes the clockwise angle of touchpoints major axis to + the surface y-axis and is normalized to the -180 to +180 degrees range. + The granuality of orientation depends on the touch device, some devices only + support binary rotation values between 0 and 90 degrees. + + This event is only sent by the compositor if the touch device supports + orientation reports. + + + + + -- 2.8.0.rc3.226.g39d4020 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 7/7] protocol: remove scaler.xml
From: Pekka Paalanen The stable version of the scaling and cropping extension is found in wayland-protocols as viewporter.xml. Remove scaler.xml as nothing uses it. Signed-off-by: Pekka Paalanen --- Makefile.am | 5 -- protocol/scaler.xml | 208 2 files changed, 213 deletions(-) delete mode 100644 protocol/scaler.xml diff --git a/Makefile.am b/Makefile.am index 546614f..2e3fc9e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -564,8 +564,6 @@ libtoytoolkit_la_SOURCES = \ nodist_libtoytoolkit_la_SOURCES = \ protocol/text-cursor-position-protocol.c\ protocol/text-cursor-position-client-protocol.h \ - protocol/scaler-protocol.c \ - protocol/scaler-client-protocol.h \ protocol/viewporter-protocol.c \ protocol/viewporter-client-protocol.h \ protocol/xdg-shell-unstable-v5-protocol.c \ @@ -771,8 +769,6 @@ BUILT_SOURCES +=\ protocol/input-method-unstable-v1-client-protocol.h \ protocol/weston-desktop-shell-client-protocol.h \ protocol/weston-desktop-shell-protocol.c\ - protocol/scaler-client-protocol.h \ - protocol/scaler-protocol.c \ protocol/viewporter-client-protocol.h \ protocol/viewporter-protocol.c \ protocol/presentation-time-protocol.c \ @@ -1361,7 +1357,6 @@ EXTRA_DIST += \ protocol/weston-screenshooter.xml \ protocol/text-cursor-position.xml \ protocol/weston-test.xml\ - protocol/scaler.xml \ protocol/ivi-application.xml\ protocol/ivi-hmi-controller.xml diff --git a/protocol/scaler.xml b/protocol/scaler.xml deleted file mode 100644 index 0e482a6..000 --- a/protocol/scaler.xml +++ /dev/null @@ -1,208 +0,0 @@ - - - - -Copyright © 2013-2014 Collabora, Ltd. - -Permission is hereby granted, free of charge, to any person obtaining a -copy of this software and associated documentation files (the "Software"), -to deal in the Software without restriction, including without limitation -the rights to use, copy, modify, merge, publish, distribute, sublicense, -and/or sell copies of the Software, and to permit persons to whom the -Software is furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice (including the next -paragraph) shall be included in all copies or substantial portions of the -Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL -THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING -FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -DEALINGS IN THE SOFTWARE. - - - - - The global interface exposing surface cropping and scaling - capabilities is used to instantiate an interface extension for a - wl_surface object. This extended interface will then allow - cropping and scaling the surface contents, effectively - disconnecting the direct relationship between the buffer and the - surface size. - - - - - Informs the server that the client will not be using this - protocol object anymore. This does not affect any other objects, - wl_viewport objects included. - - - - - - - - - - Instantiate an interface extension for the given wl_surface to - crop and scale its content. If the given wl_surface already has - a wl_viewport object associated, the viewport_exists - protocol error is raised. - - - - - - - - - - An additional interface to a wl_surface object, which allows the - client to specify the cropping and scaling of the surface - contents. - - This interface allows to define the source rectangle (src_x, - src_y, src_width, src_height) from where to take the wl_buffer - contents, and scale that to destination size (dst_width, - dst_height). This state is double-buffered, and is applied on the - next wl_surface.commit. - - The two parts of crop and scale state are independent: the source - rectangle, and the destination size. Initially both are unset, that - is, no scaling is applied. The whole of the current wl_buffer is - used as the source, and th
[PATCH weston 6/7] clients/simple-damage: migrate to wp_viewporter
From: Pekka Paalanen
Use wp_viewporter instead of wl_scaler and rename things as appropriate.
Signed-off-by: Pekka Paalanen
---
Makefile.am | 4 ++--
clients/simple-damage.c | 32
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index b5814d3..546614f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -483,8 +483,8 @@ weston_simple_shm_LDADD = $(SIMPLE_CLIENT_LIBS) libshared.la
weston_simple_damage_SOURCES = clients/simple-damage.c
nodist_weston_simple_damage_SOURCES = \
- protocol/scaler-protocol.c \
- protocol/scaler-client-protocol.h \
+ protocol/viewporter-protocol.c \
+ protocol/viewporter-client-protocol.h \
protocol/xdg-shell-unstable-v5-protocol.c \
protocol/xdg-shell-unstable-v5-client-protocol.h\
protocol/fullscreen-shell-unstable-v1-protocol.c\
diff --git a/clients/simple-damage.c b/clients/simple-damage.c
index 321b90f..82091d5 100644
--- a/clients/simple-damage.c
+++ b/clients/simple-damage.c
@@ -40,7 +40,7 @@
#include "shared/zalloc.h"
#include "xdg-shell-unstable-v5-client-protocol.h"
#include "fullscreen-shell-unstable-v1-client-protocol.h"
-#include "scaler-client-protocol.h"
+#include "viewporter-client-protocol.h"
int print_debug = 0;
@@ -49,7 +49,7 @@ struct display {
struct wl_registry *registry;
int compositor_version;
struct wl_compositor *compositor;
- struct wl_scaler *scaler;
+ struct wp_viewporter *viewporter;
struct xdg_shell *shell;
struct zwp_fullscreen_shell_v1 *fshell;
struct wl_shm *shm;
@@ -72,7 +72,7 @@ struct window {
struct display *display;
int width, height, border;
struct wl_surface *surface;
- struct wl_viewport *viewport;
+ struct wp_viewport *viewport;
struct xdg_surface *xdg_surface;
struct wl_callback *callback;
struct buffer buffers[2];
@@ -258,8 +258,8 @@ create_window(struct display *display, int width, int
height,
exit(1);
}
- if (display->scaler == NULL && (flags & WINDOW_FLAG_USE_VIEWPORT)) {
- fprintf(stderr, "Compositor does not support wl_viewport");
+ if (display->viewporter == NULL && (flags & WINDOW_FLAG_USE_VIEWPORT)) {
+ fprintf(stderr, "Compositor does not support wp_viewport");
exit(1);
}
@@ -290,8 +290,8 @@ create_window(struct display *display, int width, int
height,
window->surface = wl_compositor_create_surface(display->compositor);
if (window->flags & WINDOW_FLAG_USE_VIEWPORT)
- window->viewport = wl_scaler_get_viewport(display->scaler,
- window->surface);
+ window->viewport =
wp_viewporter_get_viewport(display->viewporter,
+ window->surface);
if (display->shell) {
window->xdg_surface =
@@ -337,7 +337,7 @@ destroy_window(struct window *window)
if (window->xdg_surface)
xdg_surface_destroy(window->xdg_surface);
if (window->viewport)
- wl_viewport_destroy(window->viewport);
+ wp_viewport_destroy(window->viewport);
wl_surface_destroy(window->surface);
free(window);
}
@@ -560,7 +560,7 @@ redraw(void *data, struct wl_callback *callback, uint32_t
time)
off_x = ty;
break;
}
- wl_viewport_set_source(window->viewport,
+ wp_viewport_set_source(window->viewport,
wl_fixed_from_int(window->width / 3),
wl_fixed_from_int(window->height / 5),
wl_fixed_from_int(window->width / 2),
@@ -640,7 +640,7 @@ redraw(void *data, struct wl_callback *callback, uint32_t
time)
window->transform);
if (window->viewport)
- wl_viewport_set_destination(window->viewport,
+ wp_viewport_set_destination(window->viewport,
window->width,
window->height);
@@ -709,9 +709,9 @@ registry_handle_global(void *data, struct wl_registry
*registry,
wl_registry_bind(registry,
id, &wl_compositor_interface,
d->compositor_version);
- } else if (strcmp(interface, "wl_scaler") == 0 && version >= 2) {
- d->scaler = wl_registry_bind(registry,
-id, &wl_scaler_interface, 2);
+ } else if (strcmp(interface, "wp_viewporter") == 0) {
+ d->viewporter = wl_reg
[PATCH weston 5/7] clients/scaler: migrate to wp_viewporter
From: Pekka Paalanen
Use wp_viewporter instead of wl_scaler and rename things accordingly.
Since interface versions were reset, there is no need to check the
interface version anymore, and the wl_scaler.set request disappeared.
Signed-off-by: Pekka Paalanen
---
Makefile.am | 4
clients/scaler.c | 43 +--
2 files changed, 17 insertions(+), 30 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 9cb0dac..b5814d3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -566,6 +566,8 @@ nodist_libtoytoolkit_la_SOURCES = \
protocol/text-cursor-position-client-protocol.h \
protocol/scaler-protocol.c \
protocol/scaler-client-protocol.h \
+ protocol/viewporter-protocol.c \
+ protocol/viewporter-client-protocol.h \
protocol/xdg-shell-unstable-v5-protocol.c \
protocol/xdg-shell-unstable-v5-client-protocol.h\
protocol/ivi-application-protocol.c \
@@ -771,6 +773,8 @@ BUILT_SOURCES +=\
protocol/weston-desktop-shell-protocol.c\
protocol/scaler-client-protocol.h \
protocol/scaler-protocol.c \
+ protocol/viewporter-client-protocol.h \
+ protocol/viewporter-protocol.c \
protocol/presentation-time-protocol.c \
protocol/presentation-time-client-protocol.h\
protocol/fullscreen-shell-unstable-v1-protocol.c\
diff --git a/clients/scaler.c b/clients/scaler.c
index 1fcf2c0..f504c73 100644
--- a/clients/scaler.c
+++ b/clients/scaler.c
@@ -32,7 +32,7 @@
#include
#include "window.h"
-#include "scaler-client-protocol.h"
+#include "viewporter-client-protocol.h"
#define BUFFER_SCALE 2
static const int BUFFER_WIDTH = 421 * BUFFER_SCALE;
@@ -50,9 +50,8 @@ struct box {
struct widget *widget;
int width, height;
- struct wl_scaler *scaler;
- int scaler_version;
- struct wl_viewport *viewport;
+ struct wp_viewporter *viewporter;
+ struct wp_viewport *viewport;
enum {
MODE_NO_VIEWPORT,
@@ -84,33 +83,20 @@ set_my_viewport(struct box *box)
src_width = wl_fixed_from_double((RECT_W - 0.5) / BUFFER_SCALE);
src_height = wl_fixed_from_double((RECT_H - 0.5) / BUFFER_SCALE);
- if (box->scaler_version < 2 && box->mode != MODE_SRC_DST) {
- fprintf(stderr, "Error: server's wl_scaler interface version "
- "%d does not support this mode.\n",
- box->scaler_version);
- exit(1);
- }
-
switch (box->mode){
case MODE_SRC_ONLY:
- wl_viewport_set_source(box->viewport, src_x, src_y,
+ wp_viewport_set_source(box->viewport, src_x, src_y,
src_width, src_height);
break;
case MODE_DST_ONLY:
- wl_viewport_set_destination(box->viewport,
+ wp_viewport_set_destination(box->viewport,
dst_width, dst_height);
break;
case MODE_SRC_DST:
- if (box->scaler_version < 2) {
- wl_viewport_set(box->viewport,
- src_x, src_y, src_width, src_height,
- dst_width, dst_height);
- } else {
- wl_viewport_set_source(box->viewport, src_x, src_y,
- src_width, src_height);
- wl_viewport_set_destination(box->viewport,
- dst_width, dst_height);
- }
+ wp_viewport_set_source(box->viewport, src_x, src_y,
+ src_width, src_height);
+ wp_viewport_set_destination(box->viewport,
+ dst_width, dst_height);
break;
default:
assert(!"not reached");
@@ -188,14 +174,11 @@ global_handler(struct display *display, uint32_t name,
{
struct box *box = data;
- if (strcmp(interface, "wl_scaler") == 0) {
- box->scaler_version = version < 2 ? version : 2;
-
- box->scaler = display_bind(display, name,
- &wl_scaler_interface,
- box->scaler_version);
+ if (strcmp(interface, "wp_viewporter") == 0) {
+ box->viewporter = display_bind(display, name,
+ &wp_viewporter_interface, 1);
- box->viewport = wl_scaler_get_viewport(box->scaler,
+ box
[PATCH weston 4/7] compositor: rename scaler to viewport(er)
From: Pekka Paalanen
Since the interface is now called wp_viewport, rename functions from
"scaler" to "viewporter" as well.
scaler_surface_to_buffer() is renamed to viewport_surface_to_buffer()
because it is more about viewport than viewporter.
Signed-off-by: Pekka Paalanen
---
src/compositor.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 551d4ed..b6a9aeb 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -877,8 +877,8 @@ weston_transformed_region(int width, int height,
}
static void
-scaler_surface_to_buffer(struct weston_surface *surface,
-float sx, float sy, float *bx, float *by)
+viewport_surface_to_buffer(struct weston_surface *surface,
+ float sx, float sy, float *bx, float *by)
{
struct weston_buffer_viewport *vp = &surface->buffer_viewport;
double src_width, src_height;
@@ -912,8 +912,8 @@ weston_surface_to_buffer_float(struct weston_surface
*surface,
{
struct weston_buffer_viewport *vp = &surface->buffer_viewport;
- /* first transform coordinates if the scaler is set */
- scaler_surface_to_buffer(surface, sx, sy, bx, by);
+ /* first transform coordinates if the viewport is set */
+ viewport_surface_to_buffer(surface, sx, sy, bx, by);
weston_transformed_coord(surface->width_from_buffer,
surface->height_from_buffer,
@@ -946,12 +946,12 @@ weston_surface_to_buffer_rect(struct weston_surface
*surface,
struct weston_buffer_viewport *vp = &surface->buffer_viewport;
float xf, yf;
- /* first transform box coordinates if the scaler is set */
- scaler_surface_to_buffer(surface, rect.x1, rect.y1, &xf, &yf);
+ /* first transform box coordinates if the viewport is set */
+ viewport_surface_to_buffer(surface, rect.x1, rect.y1, &xf, &yf);
rect.x1 = floorf(xf);
rect.y1 = floorf(yf);
- scaler_surface_to_buffer(surface, rect.x2, rect.y2, &xf, &yf);
+ viewport_surface_to_buffer(surface, rect.x2, rect.y2, &xf, &yf);
rect.x2 = ceilf(xf);
rect.y2 = ceilf(yf);
@@ -4441,25 +4441,25 @@ static const struct wp_viewport_interface
viewport_interface = {
};
static void
-scaler_destroy(struct wl_client *client,
- struct wl_resource *resource)
+viewporter_destroy(struct wl_client *client,
+ struct wl_resource *resource)
{
wl_resource_destroy(resource);
}
static void
-scaler_get_viewport(struct wl_client *client,
- struct wl_resource *scaler,
- uint32_t id,
- struct wl_resource *surface_resource)
+viewporter_get_viewport(struct wl_client *client,
+ struct wl_resource *viewporter,
+ uint32_t id,
+ struct wl_resource *surface_resource)
{
- int version = wl_resource_get_version(scaler);
+ int version = wl_resource_get_version(viewporter);
struct weston_surface *surface =
wl_resource_get_user_data(surface_resource);
struct wl_resource *resource;
if (surface->viewport_resource) {
- wl_resource_post_error(scaler,
+ wl_resource_post_error(viewporter,
WP_VIEWPORTER_ERROR_VIEWPORT_EXISTS,
"a viewport for that surface already exists");
return;
@@ -4478,14 +4478,14 @@ scaler_get_viewport(struct wl_client *client,
surface->viewport_resource = resource;
}
-static const struct wp_viewporter_interface scaler_interface = {
- scaler_destroy,
- scaler_get_viewport
+static const struct wp_viewporter_interface viewporter_interface = {
+ viewporter_destroy,
+ viewporter_get_viewport
};
static void
-bind_scaler(struct wl_client *client,
- void *data, uint32_t version, uint32_t id)
+bind_viewporter(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
{
struct wl_resource *resource;
@@ -4496,7 +4496,7 @@ bind_scaler(struct wl_client *client,
return;
}
- wl_resource_set_implementation(resource, &scaler_interface,
+ wl_resource_set_implementation(resource, &viewporter_interface,
NULL, NULL);
}
@@ -4679,7 +4679,7 @@ weston_compositor_create(struct wl_display *display, void
*user_data)
goto fail;
if (!wl_global_create(ec->wl_display, &wp_viewporter_interface, 1,
- ec, bind_scaler))
+ ec, bind_viewporter))
goto fail;
if (!wl_global_create(ec->wl_display, &wp_presentation_interface, 1,
--
2.7.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.
[PATCH wayland-protocols 1/2] stable: add viewporter draft
From: Pekka Paalanen This XML file has been copied verbatim from Weston 1.10.0 release, protocol/scaler.xml. The interfaces still need renaming according to wayland-protocols policy. Also a redundant request needs to be removed. These will be done in a follow-up patch to clearly show the changes. Signed-off-by: Pekka Paalanen --- stable/viewporter/README | 7 ++ stable/viewporter/viewporter.xml | 208 +++ 2 files changed, 215 insertions(+) create mode 100644 stable/viewporter/README create mode 100644 stable/viewporter/viewporter.xml diff --git a/stable/viewporter/README b/stable/viewporter/README new file mode 100644 index 000..e09057b --- /dev/null +++ b/stable/viewporter/README @@ -0,0 +1,7 @@ +Viewporter: cropping and scaling extension for surface contents + +Previously known as wl_scaler. + +Maintainers: +Pekka Paalanen + diff --git a/stable/viewporter/viewporter.xml b/stable/viewporter/viewporter.xml new file mode 100644 index 000..0e482a6 --- /dev/null +++ b/stable/viewporter/viewporter.xml @@ -0,0 +1,208 @@ + + + + +Copyright © 2013-2014 Collabora, Ltd. + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice (including the next +paragraph) shall be included in all copies or substantial portions of the +Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. + + + + + The global interface exposing surface cropping and scaling + capabilities is used to instantiate an interface extension for a + wl_surface object. This extended interface will then allow + cropping and scaling the surface contents, effectively + disconnecting the direct relationship between the buffer and the + surface size. + + + + + Informs the server that the client will not be using this + protocol object anymore. This does not affect any other objects, + wl_viewport objects included. + + + + + + + + + + Instantiate an interface extension for the given wl_surface to + crop and scale its content. If the given wl_surface already has + a wl_viewport object associated, the viewport_exists + protocol error is raised. + + + + + + + + + + An additional interface to a wl_surface object, which allows the + client to specify the cropping and scaling of the surface + contents. + + This interface allows to define the source rectangle (src_x, + src_y, src_width, src_height) from where to take the wl_buffer + contents, and scale that to destination size (dst_width, + dst_height). This state is double-buffered, and is applied on the + next wl_surface.commit. + + The two parts of crop and scale state are independent: the source + rectangle, and the destination size. Initially both are unset, that + is, no scaling is applied. The whole of the current wl_buffer is + used as the source, and the surface size is as defined in + wl_surface.attach. + + If the destination size is set, it causes the surface size to become + dst_width, dst_height. The source (rectangle) is scaled to exactly + this size. This overrides whatever the attached wl_buffer size is, + unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface + has no content and therefore no size. Otherwise, the size is always + at least 1x1 in surface coordinates. + + If the source rectangle is set, it defines what area of the + wl_buffer is taken as the source. If the source rectangle is set and + the destination size is not set, the surface size becomes the source + rectangle size rounded up to the nearest integer. If the source size + is already exactly integers, this results in cropping without scaling. + + The coordinate transformations from buffer pixel coordinates up to + the surface-local coordinates happen in the following order: +1. buffer_transform (wl_surface.set_buffer_transform) +2. buffer_scal
[PATCH wayland-protocols 2/2] stable/viewporter: finish stabilization
From: Pekka Paalanen Rename interfaces and the protocol to follow the policy. Reset interface versions. Remove the redundant wp_viewport.set request. Replace "surface coordinates" with "surface local coordinates". Hook up to build and install. Signed-off-by: Pekka Paalanen --- Makefile.am | 1 + stable/viewporter/viewporter.xml | 57 +++- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/Makefile.am b/Makefile.am index 033789f..71d2632 100644 --- a/Makefile.am +++ b/Makefile.am @@ -12,6 +12,7 @@ unstable_protocols = \ stable_protocols = \ stable/presentation-time/presentation-time.xml \ + stable/viewporter/viewporter.xml \ $(NULL) nobase_dist_pkgdata_DATA = \ diff --git a/stable/viewporter/viewporter.xml b/stable/viewporter/viewporter.xml index 0e482a6..1b47997 100644 --- a/stable/viewporter/viewporter.xml +++ b/stable/viewporter/viewporter.xml @@ -1,5 +1,5 @@ - + Copyright © 2013-2014 Collabora, Ltd. @@ -24,7 +24,7 @@ DEALINGS IN THE SOFTWARE. - + The global interface exposing surface cropping and scaling capabilities is used to instantiate an interface extension for a @@ -38,7 +38,7 @@ Informs the server that the client will not be using this protocol object anymore. This does not affect any other objects, - wl_viewport objects included. + wp_viewport objects included. @@ -51,18 +51,18 @@ Instantiate an interface extension for the given wl_surface to crop and scale its content. If the given wl_surface already has - a wl_viewport object associated, the viewport_exists + a wp_viewport object associated, the viewport_exists protocol error is raised. - - + An additional interface to a wl_surface object, which allows the client to specify the cropping and scaling of the surface @@ -85,7 +85,7 @@ this size. This overrides whatever the attached wl_buffer size is, unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface has no content and therefore no size. Otherwise, the size is always - at least 1x1 in surface coordinates. + at least 1x1 in surface local coordinates. If the source rectangle is set, it defines what area of the wl_buffer is taken as the source. If the source rectangle is set and @@ -97,7 +97,7 @@ the surface-local coordinates happen in the following order: 1. buffer_transform (wl_surface.set_buffer_transform) 2. buffer_scale (wl_surface.set_buffer_scale) -3. crop and scale (wl_viewport.set*) +3. crop and scale (wp_viewport.set*) This means, that the source rectangle coordinates of crop and scale are given in the coordinates after the buffer transform and scale, i.e. in the coordinates that would be the surface-local coordinates @@ -113,10 +113,10 @@ still in the surface-local coordinate system, just like dst_width and dst_height are. - If the wl_surface associated with the wl_viewport is destroyed, - the wl_viewport object becomes inert. + If the wl_surface associated with the wp_viewport is destroyed, + the wp_viewport object becomes inert. - If the wl_viewport object is destroyed, the crop and scale + If the wp_viewport object is destroyed, the crop and scale state is removed from the wl_surface. The change will be applied on the next wl_surface.commit. @@ -133,37 +133,10 @@ summary="negative or zero values in width or height"/> - - - Set both source rectangle and destination size of the associated - wl_surface. See wl_viewport for the description, and relation to - the wl_buffer size. - - The bad_value protocol error is raised if src_width or - src_height is negative, or if dst_width or dst_height is not - positive. - - The crop and scale state is double-buffered state, and will be - applied on the next wl_surface.commit. - - Arguments dst_x and dst_y do not exist here, use the x and y - arguments to wl_surface.attach. The x, y, dst_width, and dst_height - define the surface-local coordinate system irrespective of the - attached wl_buffer size. - - - - - - - - - - - + Set the source rectangle of the associated wl_surface. See - wl_viewport for the description, and relation to the wl_buffer + wp_viewport for the description, and relation to the wl_buffer size. If widt
[PATCH wayland-protocols weston 0/7] Stabilize wl_scaler as wp_viewporter
From: Pekka Paalanen Hi, it's a high time we declare the scaling and cropping extension as stable. I took wl_scaler from Weston, renamed it to wp_viewporter, and removed the redundant wp_viewport.set request. There must be a wayland-protocols release, before the Weston patches can be landed, and when that happens, the wayland-protocols dependency must be bumped accordingly. Thanks, pq Pekka Paalanen (2): stable: add viewporter draft stable/viewporter: finish stabilization Makefile.am | 1 + stable/viewporter/README | 7 ++ stable/viewporter/viewporter.xml | 181 +++ 3 files changed, 189 insertions(+) create mode 100644 stable/viewporter/README create mode 100644 stable/viewporter/viewporter.xml Pekka Paalanen (5): compositor: migrate to stable viewporter.xml compositor: rename scaler to viewport(er) clients/scaler: migrate to wp_viewporter clients/simple-damage: migrate to wp_viewporter protocol: remove scaler.xml Makefile.am | 17 ++-- clients/scaler.c| 43 +++--- clients/simple-damage.c | 32 configure.ac| 2 + protocol/scaler.xml | 208 src/compositor.c| 110 - src/compositor.h| 5 +- 7 files changed, 76 insertions(+), 341 deletions(-) delete mode 100644 protocol/scaler.xml -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 3/7] compositor: migrate to stable viewporter.xml
From: Pekka Paalanen
Migrate from wl_scaler to wp_viewporter extension. The viewporter.xml
file is provided by wayland-protocols.
This stops Weston from advertising wl_scaler, and advertises
wp_viewporter instead.
Signed-off-by: Pekka Paalanen
---
Makefile.am | 4 ++--
configure.ac | 2 ++
src/compositor.c | 68 +++-
src/compositor.h | 5 +++--
4 files changed, 20 insertions(+), 59 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 9bed32c..9cb0dac 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -124,8 +124,8 @@ nodist_weston_SOURCES =
\
protocol/input-method-unstable-v1-server-protocol.h \
protocol/presentation-time-protocol.c \
protocol/presentation-time-server-protocol.h\
- protocol/scaler-protocol.c \
- protocol/scaler-server-protocol.h \
+ protocol/viewporter-protocol.c \
+ protocol/viewporter-server-protocol.h \
protocol/linux-dmabuf-unstable-v1-protocol.c\
protocol/linux-dmabuf-unstable-v1-server-protocol.h
diff --git a/configure.ac b/configure.ac
index 447cf6b..6f7b349 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4,6 +4,8 @@ m4_define([weston_micro_version], [90])
m4_define([weston_version],
[weston_major_version.weston_minor_version.weston_micro_version])
+# XXX: bump wayland-protocols dependency to include viewporter.xml
+
AC_PREREQ([2.64])
AC_INIT([weston],
[weston_version],
diff --git a/src/compositor.c b/src/compositor.c
index 5500197..551d4ed 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -54,7 +54,7 @@
#include "timeline.h"
#include "compositor.h"
-#include "scaler-server-protocol.h"
+#include "viewporter-server-protocol.h"
#include "presentation-time-server-protocol.h"
#include "shared/helpers.h"
#include "shared/os-compatibility.h"
@@ -923,7 +923,7 @@ weston_surface_to_buffer_float(struct weston_surface
*surface,
/** Transform a rectangle from surface coordinates to buffer coordinates
*
- * \param surface The surface to fetch wl_viewport and buffer transformation
+ * \param surface The surface to fetch wp_viewport and buffer transformation
* from.
* \param rect The rectangle to transform.
* \return The transformed rectangle.
@@ -963,7 +963,7 @@ weston_surface_to_buffer_rect(struct weston_surface
*surface,
/** Transform a region from surface coordinates to buffer coordinates
*
- * \param surface The surface to fetch wl_viewport and buffer transformation
+ * \param surface The surface to fetch wp_viewport and buffer transformation
* from.
* \param surface_region[in] The region in surface coordinates.
* \param buffer_region[out] The region converted to buffer coordinates.
@@ -2826,7 +2826,8 @@ weston_surface_commit_state(struct weston_surface
*surface,
/* wl_surface.set_buffer_transform */
/* wl_surface.set_buffer_scale */
- /* wl_viewport.set */
+ /* wp_viewport.set_source */
+ /* wp_viewport.set_destination */
surface->buffer_viewport = state->buffer_viewport;
/* wl_surface.attach */
@@ -4365,48 +4366,6 @@ viewport_destroy(struct wl_client *client,
}
static void
-viewport_set(struct wl_client *client,
-struct wl_resource *resource,
-wl_fixed_t src_x,
-wl_fixed_t src_y,
-wl_fixed_t src_width,
-wl_fixed_t src_height,
-int32_t dst_width,
-int32_t dst_height)
-{
- struct weston_surface *surface =
- wl_resource_get_user_data(resource);
-
- assert(surface->viewport_resource != NULL);
-
- if (wl_fixed_to_double(src_width) < 0 ||
- wl_fixed_to_double(src_height) < 0) {
- wl_resource_post_error(resource,
- WL_VIEWPORT_ERROR_BAD_VALUE,
- "source dimensions must be non-negative (%fx%f)",
- wl_fixed_to_double(src_width),
- wl_fixed_to_double(src_height));
- return;
- }
-
- if (dst_width <= 0 || dst_height <= 0) {
- wl_resource_post_error(resource,
- WL_VIEWPORT_ERROR_BAD_VALUE,
- "destination dimensions must be positive (%dx%d)",
- dst_width, dst_height);
- return;
- }
-
- surface->pending.buffer_viewport.buffer.src_x = src_x;
- surface->pending.buffer_viewport.buffer.src_y = src_y;
- surface->pending.buffer_viewport.buffer.src_width = src_width;
- surface->pending.buffer_viewport.buffer.src_height = src_height;
- surface->pending.buffer_viewport.surface.width = dst_width;
- surface->pending.buffer_viewport.surface.height = dst_height;
- surface->pending.buffer_viewport.changed = 1;
-}
-
-static void
viewport_set_source(s
Re: [PATCH weston v5 03/11] drm: port the drm backend to the new init api
On Fri, 15 Apr 2016 14:32:04 +0200 Benoit Gschwind wrote: > Hello Bryce and Giulio, > > Thanks for your contribution :), here is my comments. > > Le 13/04/2016 12:25, Bryce Harrington a écrit : > > From: Giulio Camuffo > > > > Signed-off-by: Bryce Harrington > > Reviewed-by: Quentin Glidic > > Acked-by: Pekka Paalanen > > > > --- > > v5: > > - Update to reflect format rename to gdb_format > > - Initialize width/height (suggested by pq) > > - squash drm structure versioning (suggested by pq) > > > > Makefile.am | 3 + > > src/compositor-drm.c | 197 > > --- > > src/compositor-drm.h | 111 + > > src/compositor.h | 2 - > > src/main.c | 96 - > > 5 files changed, 283 insertions(+), 126 deletions(-) > > create mode 100644 src/compositor-drm.h > > @@ -3095,18 +3053,18 @@ drm_backend_create(struct weston_compositor > > *compositor, > > */ > > b->sprites_are_broken = 1; > > b->compositor = compositor; > > + b->use_pixman = config->use_pixman; > > Duplicate configuration variable, while config structure is kept during > the backend life (line below), just keep the config->use_pixman. > > > + b->config = config; > > When I did this with headless and x11 drm, a question raised: "Should I > copy or should I take the ownership ?". Taking the ownership leave to > the developer the possibility to change the configuration easily while > running. Maybe some documentation may explicit that it's forbidden to > change configuration after run is started. Hi, a later patch removes that assignment. Changing configuration by simply changing a value without notifying the backend is going to end up in tears. If anything needs to be changeable, it must be done with a function call. Btw. the plugin registry RFC I sent a while ago might offer a nice way for the compositor to access backend-specific functions, if the backend registers an API. Thanks, pq pgpqr79LlNTU1.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 v5 00/11] drm: port the drm backend to the new init api
Hello Bryce, The patches set was tested above [1] I tested drm-bakcend and x11-backend successfully with the full patchset. Here are the resume of my comments and review. I send separate e-mail when contextual comments are required. PATCH 01/11: Do what is expected. Reviewed-by: Benoit Gschwind PATCH 02/11: Do what is expected. Reviewed-by: Benoit Gschwind PATCH 03/11 Already commented in another e-mail PATCH 04/11 Do what is expected. Reviewed-by: Benoit Gschwind PATCH 05/11 Do what is expected. Reviewed-by: Benoit Gschwind PATCH 06/11 Agree with Pekka comments, I'm available to fix issues if needed. PATCH 07/11 Agree with Pekka comments, I'm available to fix issues if needed. PATCH 08/11 The re-factor look functional. Other decision tie to the community :) PATCH 09/11 This patch look to address some concern I raised in the patch 03/11, but seems to need more discussion. PATCH 10/11 The patch look to address issue of PATCH 07/11 that Pekka raised about freeing config variable. PATCH 11/11 Look fine. Reviewed-by: Benoit Gschwind [1] git node: 94cb06a208130b0ee16553a2cd513e5e7d67f368 Le 13/04/2016 12:25, Bryce Harrington a écrit : > In following up on my earlier update of Giulio's drm backend config > patch, I've taken the liberty to try and also integrate a couple of > Benoit's other backend configuration patches into this patchset. > > Giulio and Benoit took different approaches in their implementations. > I've attempted to reconcile them so that they are stylistically > consistent, along with incorporating pq's request to have the root > config structure incorporate versioning information. I went through all > the review comments against each of the backend config patches and > incorporated the suggestions people made. The resultant changes have > ended up a bit more extensive than I had expected, and I apologize for > any toe stepping I'm doing but my hope is to move the collective work > closer to being landable. > > --- > todo: > - Use backend-specific header #defines for struct_version values > v5: > - Add missing compositor-drm.h > - Integrate wayland, x11, and headless backend patches > - Implement struct versioning for wayland, x11, and headless backend > patches in the weston_backend_config structure, as requested by pq > - Drop bzero usage as suggested in review by Jan Engelhardt > - Don't change 'backend_init' entry point function name, as suggested > in review by Giulio > - Prefer use of load_backend_new() for actual module loading, as > suggested in Giulio's review of the x11 backend patch > - Refactor all backend initialization code paths and code style for > consistency. > - Squashed drm config struct versioning with drm patch; remainder moved > to a prerequisite patch. > v4: > - Update to current trunk > - Add missing param doc for mode in drm_output_choose_initial_mode > - Rebase to account for code changes by 91880f1e to make vt > switching configurable. > > > > Benoit Gschwind (1): > x11: port the x11 backend to the new init api > > Bryce Harrington (9): > Revert "main: Remove unused function load_backend_new()" > compositor: Version the backend configuration structures > drm: Fix gcc warning about missing braces. > compositor: Document refs for alternatives/assumptions for backend > configs > headless: port the headless backend to the new init api > drm: Code and comments reformatting for consistency with other backend > configs > drm: Don't hang onto the backend config object post-backend_init > Enforce destruction of all backend config objects after initialization > drm: Drop use of drm_config config wrapper > > Giulio Camuffo (1): > drm: port the drm backend to the new init api > > Makefile.am | 5 + > src/compositor-drm.c | 216 ++-- > src/compositor-drm.h | 125 +++ > src/compositor-headless.c | 69 +-- > src/compositor-headless.h | 51 > src/compositor-x11.c | 157 > src/compositor-x11.h | 60 + > src/compositor.h | 24 +++- > src/main.c| 304 > +- > 9 files changed, 734 insertions(+), 277 deletions(-) > create mode 100644 src/compositor-drm.h > create mode 100644 src/compositor-headless.h > create mode 100644 src/compositor-x11.h > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v5 03/11] drm: port the drm backend to the new init api
Hello Bryce and Giulio,
Thanks for your contribution :), here is my comments.
Le 13/04/2016 12:25, Bryce Harrington a écrit :
> From: Giulio Camuffo
>
> Signed-off-by: Bryce Harrington
> Reviewed-by: Quentin Glidic
> Acked-by: Pekka Paalanen
>
> ---
> v5:
> - Update to reflect format rename to gdb_format
> - Initialize width/height (suggested by pq)
> - squash drm structure versioning (suggested by pq)
>
> Makefile.am | 3 +
> src/compositor-drm.c | 197
> ---
> src/compositor-drm.h | 111 +
> src/compositor.h | 2 -
> src/main.c | 96 -
> 5 files changed, 283 insertions(+), 126 deletions(-)
> create mode 100644 src/compositor-drm.h
>
> diff --git a/Makefile.am b/Makefile.am
> index d1644ac..f4cff4c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -72,6 +72,7 @@ weston_SOURCES =\
> src/log.c \
> src/compositor.c\
> src/compositor.h\
> + src/compositor-drm.h\
> src/input.c \
> src/data-device.c \
> src/screenshooter.c \
> @@ -207,6 +208,7 @@ westonincludedir = $(includedir)/weston
> westoninclude_HEADERS = \
> src/version.h \
> src/compositor.h\
> + src/compositor-drm.h\
> src/timeline-object.h \
> shared/matrix.h \
> shared/config-parser.h \
> @@ -276,6 +278,7 @@ drm_backend_la_CFLAGS = \
> $(AM_CFLAGS)
> drm_backend_la_SOURCES = \
> src/compositor-drm.c\
> + src/compositor-drm.h\
> $(INPUT_BACKEND_SOURCES)\
> shared/helpers.h\
> shared/timespec-util.h \
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 119b6c5..508b7e8 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -50,6 +50,7 @@
> #include "shared/timespec-util.h"
> #include "libbacklight.h"
> #include "compositor.h"
> +#include "compositor-drm.h"
> #include "gl-renderer.h"
> #include "pixman-renderer.h"
> #include "libinput-seat.h"
> @@ -74,17 +75,6 @@
> #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> #endif
>
> -static int option_current_mode = 0;
> -
> -enum output_config {
> - OUTPUT_CONFIG_INVALID = 0,
> - OUTPUT_CONFIG_OFF,
> - OUTPUT_CONFIG_PREFERRED,
> - OUTPUT_CONFIG_CURRENT,
> - OUTPUT_CONFIG_MODE,
> - OUTPUT_CONFIG_MODELINE
> -};
> -
> struct drm_backend {
> struct weston_backend base;
> struct weston_compositor *compositor;
> @@ -130,6 +120,8 @@ struct drm_backend {
>
> int32_t cursor_width;
> int32_t cursor_height;
> +
> + struct weston_drm_backend_config *config;
> };
>
> struct drm_mode {
> @@ -220,13 +212,6 @@ struct drm_sprite {
> uint32_t formats[];
> };
>
> -struct drm_parameters {
> - int connector;
> - int tty;
> - int use_pixman;
> - const char *seat_id;
> -};
> -
> static struct gl_renderer_interface *gl_renderer;
>
> static const char default_seat[] = "seat0";
> @@ -2151,31 +2136,23 @@ setup_output_seat_constraint(struct drm_backend *b,
> }
>
> static int
> -get_gbm_format_from_section(struct weston_config_section *section,
> - uint32_t default_value,
> - uint32_t *format)
> +parse_gbm_format(const char *s, uint32_t default_value, uint32_t *gbm_format)
> {
> - char *s;
> int ret = 0;
>
> - weston_config_section_get_string(section,
> - "gbm-format", &s, NULL);
> -
> if (s == NULL)
> - *format = default_value;
> + *gbm_format = default_value;
> else if (strcmp(s, "xrgb") == 0)
> - *format = GBM_FORMAT_XRGB;
> + *gbm_format = GBM_FORMAT_XRGB;
> else if (strcmp(s, "rgb565") == 0)
> - *format = GBM_FORMAT_RGB565;
> + *gbm_format = GBM_FORMAT_RGB565;
> else if (strcmp(s, "xrgb2101010") == 0)
> - *format = GBM_FORMAT_XRGB2101010;
> + *gbm_format = GBM_FORMAT_XRGB2101010;
> else {
> weston_log("fatal: unrecognized pixel format: %s\n", s);
> ret = -1;
> }
>
> - free(s);
> -
> return ret;
> }
>
> @@ -2194,21 +2171,38 @@ get_gbm_format_from_section(struct
> weston_config_section *section,
> * @returns A mode from the output's mode list, or NULL if none available
> */
> static struct drm_mode *
> -drm_output_choose_initial_mode(str
Re: [PATCH v2 libinput 0/5] Add tablet pad support
Hi!, On Mon, Apr 11, 2016 at 5:15 AM, Peter Hutterer wrote: > > Second version of the tablet pad support patches. The main change is > switching from button codes to simple button numbers. This is motivated > by the fact that most buttons don't have any actual meaning and the only > reason we have something other than BTN_0, BTN_1 etc is that we run out of > space in the kernel's event code range. What a button does is defined > largely by the client anyway. > > This also caused the removal of the seat button count and a change from > libinput_event_tablet_pad_get_button() to the more explicitly named > libinput_event_tablet_pad_get_button_number(). Just to drive the point home. The whole series look great to me, just the small glitch I pointed out in 1/5. Other than that, the whole series is: Reviewed-by: Carlos Garnacho Cheers, Carlos ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2 libinput 1/5] tablet: move the libwacom check for left-handed-ness into a helper function
Hey!,
On Mon, Apr 11, 2016 at 5:15 AM, Peter Hutterer
wrote:
> Signed-off-by: Peter Hutterer
> ---
> Changes since v1:
> - new in this version
>
> src/evdev-tablet.c | 38 +-
> src/evdev.c| 52
> src/evdev.h| 3 +++
> 3 files changed, 56 insertions(+), 37 deletions(-)
>
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 9a1ac52..3c3ebcf 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -1571,45 +1571,9 @@ tablet_init_accel(struct tablet_dispatch *tablet,
> struct evdev_device *device)
> static void
> tablet_init_left_handed(struct evdev_device *device)
> {
> -#if HAVE_LIBWACOM
> - struct libinput *libinput = device->base.seat->libinput;
> - WacomDeviceDatabase *db;
> - WacomDevice *d = NULL;
> - WacomError *error;
> - const char *devnode;
> -
> - db = libwacom_database_new();
> - if (!db) {
> - log_info(libinput,
> -"Failed to initialize libwacom context.\n");
> - return;
> - }
> - error = libwacom_error_new();
> - devnode = udev_device_get_devnode(device->udev_device);
> -
> - d = libwacom_new_from_path(db,
> - devnode,
> - WFALLBACK_NONE,
> - error);
> -
> - if (d) {
> - if (libwacom_is_reversible(d))
> + if (evdev_tablet_has_left_handed(device))
> evdev_init_left_handed(device,
>tablet_change_to_left_handed);
> - } else if (libwacom_error_get_code(error) == WERROR_UNKNOWN_MODEL) {
> - log_info(libinput, "Tablet unknown to libwacom\n");
> - } else {
> - log_error(libinput,
> - "libwacom error: %s\n",
> - libwacom_error_get_message(error));
> - }
> -
> - if (error)
> - libwacom_error_free(&error);
> - if (d)
> - libwacom_destroy(d);
> - libwacom_database_destroy(db);
> -#endif
> }
>
> static int
> diff --git a/src/evdev.c b/src/evdev.c
> index 6bb8986..a5511c5 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -43,6 +43,10 @@
> #include "filter.h"
> #include "libinput-private.h"
>
> +#if HAVE_LIBWACOM
> +#include
> +#endif
> +
> #define DEFAULT_WHEEL_CLICK_ANGLE 15
> #define DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT ms2us(200)
>
> @@ -2858,3 +2862,51 @@ evdev_device_destroy(struct evdev_device *device)
> free(device->mt.slots);
> free(device);
> }
> +
> +bool
> +evdev_tablet_has_left_handed(struct evdev_device *device)
> +{
> +#if HAVE_LIBWACOM
> + struct libinput *libinput = device->base.seat->libinput;
> + WacomDeviceDatabase *db;
> + WacomDevice *d = NULL;
> + WacomError *error;
> + const char *devnode;
> + bool has_left_handed = false;
> +
> + db = libwacom_database_new();
> + if (!db) {
> + log_info(libinput,
> +"Failed to initialize libwacom context.\n");
> + goto out;
> + }
> +
> + error = libwacom_error_new();
> + devnode = udev_device_get_devnode(device->udev_device);
> +
> + d = libwacom_new_from_path(db,
> + devnode,
> + WFALLBACK_NONE,
> + error);
> +
> + if (d) {
> + if (libwacom_is_reversible(d))
> + has_left_handed = true;
> + } else if (libwacom_error_get_code(error) == WERROR_UNKNOWN_MODEL) {
> + log_info(libinput, "Tablet unknown to libwacom\n");
> + } else {
> + log_error(libinput,
> + "libwacom error: %s\n",
> + libwacom_error_get_message(error));
> + }
> +
> + if (error)
> + libwacom_error_free(&error);
> + if (d)
> + libwacom_destroy(d);
> + libwacom_database_destroy(db);
> +
> +out:
> + return has_left_handed;
> +#endif
> +}
This function should have a return value (I guess false is safe?) if
HAVE_LIBWACOM is not defined.
Cheers,
Carlos
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v3] protocol: Extend wl_touch with touchpoint shape event
On Apr 14, 2016, at 8:54 PM, Jonas Ådahl wrote: > > On Thu, Apr 14, 2016 at 07:04:25PM -0500, Yong Bakos wrote: >> On Apr 14, 2016, at 3:23 PM, Dennis Kempin wrote: >>> >>> This CL updates the wl_touch interface with a shape and >>> orientation event. >>> The shape/orientation of a touch point is not relevant for most UI >>> applications, but allows a better experience in some cases >>> such as drawing apps. >>> >>> The events are used by the compositor to inform the client >>> about changes in the shape and orientation of a touchpoint, which is >>> approximated by an ellipse and it's angle to the y-axis. >>> >>> The event is optional and only sent when compositor and the >>> touch device support this type of information. The client is >>> responsible for making a reasonable assumption about the >>> touch shape if no shape is reported. >>> >>> Signed-off-by: Dennis Kempin >> >> Hi Dennis, >> I've noted some concerns inline below. I'm a bit inexperienced here, so >> please do correct/educate me where I'm wrong. >> >> >>> --- >>> protocol/wayland.xml | 75 >>> >>> 1 file changed, 64 insertions(+), 11 deletions(-) >>> >>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml >>> index 12a6efd..d19d898 100644 >>> --- a/protocol/wayland.xml >>> +++ b/protocol/wayland.xml >>> @@ -910,11 +910,6 @@ >>> copy-and-paste and drag-and-drop. These mechanisms are tied to >>> a wl_seat and this interface lets a client get a wl_data_device >>> corresponding to a wl_seat. >>> - >>> - Depending on the version bound, the objects created from the bound >>> - wl_data_device_manager object will have different requirements for >>> - functioning properly. See wl_data_source.set_actions, >>> - wl_data_offer.accept and wl_data_offer.finish for details. >> >> What does the deletion of this paragraph have to do with adding the >> two new touchpoint shape events? >> >> >>> >>> >>> >>> @@ -1656,7 +1651,7 @@ >>> >>> >>> >>> - >>> + >> >> Why are you bumping this instead of just bumping the version of the >> wl_touch interface? > > Hi, > > Just going to reply to your comments regarding the version bumps. > > In Wayland, objects (except wl_display) are created from some other > object, and most "concepts" (such as input, surfaces, data > device/clipboard) have a single "global" object where all the other > objects are created from. > > When you create an object from another object, except for global > objects, the new object will inherit the version of the object it was > created from. So for example if you have a global wl_compositor object > with version N, all wl_surface's created from that object will also have > the version N. If you need to bump the version of wl_surface, if you > want to be able to create an object with the bumped version, you also > need to bump the version of the interface the wl_surface is created > from, i.e. wl_compositor. > > In this patch, the interface bumped is wl_touch, and the interface of > the object a wl_touch object is created from is wl_seat. Thus in order > to be able to get a wl_touch object with the bumped version, you also > need to bump the version of each parent interface up to the global > object. > > Note that when you bump the version of the interface of the global > object, when creating an object from that global, it'll inherit the > version. This means that when you bump wl_touch and wl_seat, you'll > effectively get wl_pointer and wl_keyboard objects with the bumped > version as well. > > While it is required to bump the parents up to the global object, it is > completely optional to actually bump the interface versions of all the > other interfaces in an "interface tree". The reason for doing so may be > to make it clear the actual highest version an object created with this > interface may be created at, even though no new events nor requests were > added for some particular version. > > Hope this cleares things up. Ah - I was suspecting that. /Thank you/ for taking the time to explain! I swear I've read the documentation, which mentions this in the Versioning section, but it didn't stick in my mind. yong > > Jonas > >> >> >>> >>> A seat is a group of keyboards, pointer and touch devices. This >>> object is published as a global during start up, or when such a >>> @@ -1765,7 +1760,7 @@ >>> >>> >>> >>> - >>> + >> >> Why are you bumping this instead of just bumping the version of the >> wl_touch interface? >> >> >>> >>> The wl_pointer interface represents one or more input devices, >>> such as mice, which control the pointer location and pointer_focus >>> @@ -2038,7 +2033,7 @@ >>> >>> The timestamp is to be interpreted identical to the timestamp in the >>> wl_pointer.axis event. The timestamp value may be the same as a >>> - preceding wl_pointer.axis event. >>> + preceeding wl_pointer.axis event. >> >>
Re: [PATCH wayland-protocols 7/7] xdg-shell: Introduce xdg_positioner
On Thu, Apr 14, 2016 at 09:33:38AM -0500, Yong Bakos wrote: > On Apr 14, 2016, at 3:28 AM, Jonas Ådahl wrote: > > > > xdg_positioner is a method for declarative positioning of child surfaces > > (currently only xdg_popup surfaces). A client creates a description of a > > positioning logic using the xdg_positioner interface. The xdg_positioner > > object is then used when creating a xdg_popup for describing how the > > child surface should be positioned in relation to the parent surface. > > > > Signed-off-by: Jonas Ådahl > > Signed-off-by: Mike Blumenkrantz > > Hi, > My last mildly-annoying corrections to this patchset are inline, below. > Thanks for putting up with me. :) Not annoying at all. Thanks for the careful review! I will fix things you brought up locally. Jonas > > yong > > > > --- > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 206 > > ++- > > 1 file changed, 204 insertions(+), 2 deletions(-) > > > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > index a2a6f12..2b51802 100644 > > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > @@ -57,6 +57,15 @@ > > > > > > > > + > > + > > + Create a positioner object. A positioner object is used to position > > + surfaces relative to some parent surface. See the interface description > > + for details. > > + > > + > > + > > + > > > > > > This creates an xdg_surface for the given surface. While xdg_surface > > @@ -96,6 +105,186 @@ > > > > > > > > + > > + > > + The xdg_positioner provides an interface for constructing positioning > > + rules used for positioning a child surface relative to another > > surface > > + in a certain way. It allows methods for defining a rule that will > > make > > + the child surface stay within the border of the visible area of the > > + screen, with different ways in which the child surface should change > > + its position, including sliding along an axis, or flipping around a > > + rectangle. > > + > > + See the various requests for details about possible rules. > > + > > + An xdg_positioner object may be re-used when positioning different > > + surfaces until destroyed using xdg_positioner.destroy. > > + > > + > > + > > + > > + > > + > > + > > + > > + Notify the compositor that the xdg_positioner will no longer be used. > > + > > + > > + > > + > > + > > + Specify the anchor rectangle of the parent surface that the child > > + surface will be placed relative to. The rectangle is relative to the > > + window geometry as defined by xdg_surface.set_window_geometry of the > > + parent surface. The rectangle must be at least be 1x1 large. > > must be at least 1x1 large. > > > > + > > + When used to position a child surface, the attach rectangle may not > > + extend outside of the window geometry of the parent surface. > > + > > + > > + > > + > > + > > + > > + > > + > > + > +summary="the center of the anchor rectangle"/> > > + > +summary="the left edge of the anchor rectangle"/> > > + > +summary="the right edge of the anchor rectangle"/> > > + > +summary="the top edge of the anchor rectangle"/> > > + > +summary="the bottom edge of the anchor rectangle"/> > > + > > + > > + > > + > > + Set the anchor edges of the anchor rectangle. The anchor edges > > + defines where on the anchor rectangle the child surface should > > define where ... should be > > > > + positioned relative to. An anchor is a bit mask of zero to two values of > > + the anchor enum. Two values on the same axis (for example left and > > + right) may not be combined. > > + > > + If no anchor is set on any axis, the anchor position will be positioned > > + at the center of the anchor rectangle on the unset axis. The default > > + value is "none". > > + > > + > + summary="bit mask of anchor edges"/> > > + > > + > > + > > + > +summary="center above the anchor position"/> > > + > +summary="position to the left of the anchor position"/> > > + > +summary="position to the right of the anchor position"/> > > + > +summary="position above the anchor position"/> > > + > +summary="position below the anchor position"/> > > + > > + > > + > > + > > + The gravity defines in what direction a surface would be positioned, > > + relative to the anchor position on the parent surface. A gravity > > + is a bit mask of zero to two values of the gravity enum. Two values > > + on the same axis (for example left and right) may not be combined. > > + > > + If no gravity is set on an axis, t
Re: [PATCH 2/5] drm: port the drm backend to the new init api
On Thu, 14 Apr 2016 19:09:34 +0300
Giulio Camuffo wrote:
> 2016-04-13 14:30 GMT+03:00 Pekka Paalanen :
> > On Tue, 12 Apr 2016 21:34:28 -0700
> > Bryce Harrington wrote:
> >
> >> On Wed, Apr 06, 2016 at 11:37:57AM +0300, Pekka Paalanen wrote:
> >> > On Wed, 9 Mar 2016 16:49:29 -0800
> >> > Bryce Harrington wrote:
> >> >
> >> > > From: Giulio Camuffo
> >> > >
> >> > > Signed-off-by: Bryce Harrington
> >> > > Reviewed-by: Quentin Glidic
> >> > > Acked-by: Pekka Paalanen
> >> > > ---
> >> > > v4: Update to current trunk
> >> > > - Add missing param doc for mode in drm_output_choose_initial_mode
> >> > > - Rebase to account for code changes by 91880f1e to make vt
> >> > > switching configurable.
> >> > >
> >> > > Makefile.am | 3 +
> >> > > src/compositor-drm.c | 196
> >> > > ++-
> >> > > src/compositor.h | 2 -
> >> > > src/main.c | 94 +++-
> >> > > 4 files changed, 165 insertions(+), 130 deletions(-)
> >> >
> >> > Hi Giulio and Bryce,
> >> >
> >> > I'm sorry it has taken so long for me to come back to this.
> >
> >> > > +static int
> >> > > +load_drm_backend(struct weston_compositor *c, const char *backend,
> >> > > + int *argc, char **argv, struct weston_config *wc)
> >> > > +{
> >> > > + struct drm_config *config;
> >> > > + struct weston_config_section *section;
> >> > > + int ret = 0;
> >> > > +
> >> > > + config = zalloc(sizeof *config);
> >> > > + if (!config)
> >> > > + return -1;
> >> >
> >> > This function will be affected by the struct versioning too.
> >>
> >> The subsequent patch adds the versioning, although later in the routine
> >> than here.
> >>
> >> > > + const struct weston_option options[] = {
> >> > > + { WESTON_OPTION_INTEGER, "connector", 0,
> >> > > &config->base.connector },
> >> > > + { WESTON_OPTION_STRING, "seat", 0, &config->base.seat_id },
> >> > > + { WESTON_OPTION_INTEGER, "tty", 0, &config->base.tty },
> >> > > + { WESTON_OPTION_BOOLEAN, "current-mode", 0,
> >> > > + &config->use_current_mode },
> >> > > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0,
> >> > > &config->base.use_pixman },
> >> > > + };
> >> >
> >> > Mixed declarations and code.
> >>
> >> Hmm, I'm not sure best how to address this. Options is being
> >> initialized with pointers that are created by the zalloc, so I don't
> >> think I can just separate the declaration from the initialization. Does
> >> options need to be changed to be dynamically allocated as well?
> >
> > Hi,
> >
> > I suppose the simplest solution is to make another function, define
> > 'options[]' in it, and get 'config' as an argument. Then it can call
> > the parser and return. Something like ...fill_from_command_line(struct
> > drm_config *config, argc, argv...).
> >
> > It's fine to keep your changes as separate patches as long as every
> > patch is bisectable.
> >
> > I'm going through the new series right now, and noticed these emails
> > from you just now.
>
> I think this is one of the cases where strict style compliance goes in
> the way of better code. Adding a function just adds an indirect level
> but gives nothing in return, because it would really be just a wapper
> around the options[] variable. I don't see it much different to a "int
> value_1() { return 1; }".
> But more importantly imho, it requires another revision.
No, it would be much more. The function would essentially be "fill in
this config struct from these argc,argv". That color would actually
please my eye more, as that is a logical single step to execute from a
load_*() function, rather than stuffing it all inline in load_*().
But, I'm not the one building the shed here. If I really care, I can
fix it after landing this stuff, since it doesn't seem to break the
build.
Thanks,
pq
pgpBXXpQpCFKO.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] option-parser: Handle short double-arg options
On Thu, 14 Apr 2016 11:10:57 -0700
Bryce Harrington wrote:
> On Thu, Apr 14, 2016 at 03:16:07PM +0300, Pekka Paalanen wrote:
> > On Sat, 13 Feb 2016 23:56:38 +0100
> > Benoit Gschwind wrote:
> >
> > > Hello Bryce,
> > >
> > > It seems the corner case '-f42xxx 2938475' doesn't work as expected with
> > > 'f' short option as integer:
> > >
> > > 1. one dash then call short_option
> > > 2. in short_option will check arg[2] and call handle_option
> > > 3. in handle_option will call strtol, where *value is '4' and *p is 'x'
> > > thus *value && !*p is 0
> > > 4. return from handle_option with 0
> > > 5. return from short_option with 0
> > > 6. call short_option_with_arg with arg = "-f42xxx" and param = "2938475"
> > > 7. pass through all test and call handle_option
> > > 8. give 2938475 to the option
> > >
> > > The expected behavior is an error.
> > >
> > > Should I use Reviewed-by ?
> >
> > No, you only give Reviewed-by when you think everything is good. You
> > are pointing out a problem instead.
> >
> > Looks like this patch landed. Isn't there something to fix?
>
> The option handling code is relatively concise, which is good but the
> consequence is that there's a heap of corner cases like the above.
>
> Personally I don't think it's worth going overboard in trying to solve
> every potential case, just ones that are likely to come up in use,
> because if we do want a really robust option parsing system we probably
> should be looking at one of the several widely used option handling
> libraries like popt or whatnot.
>
Hi Bryce,
in that case, shouldn't we just revert this patch?
> > I think we might have tests for the option parser. Would be good to add
> > this particular corner-case as a test there. If we don't have tests,
> > would be nice to have some.
>
> Of course I'm a strong +1 for more tests, but again I think if we care
> that much about making the option handling solid, we really ought first
> do some due diligence looking at existing solutions. This isn't an area
> where we're value-adding much...
>
> Either way we go, I'd be happy to integrate a replacement lib or improve
> our existing solution, although I have some other matters that would be
> higher priority in the near term.
Personally I've been ok with what it was. I have forgotten why it
didn't originally just wrap getopt_long (a GNUism? too much effort?).
But you're right, it's a really unimportant thing, so I won't push it
in any direction myself. It just looked like you ignored a review
comment pointing out a new problem.
Changing the command line argument syntax might even be too disruptive
nowadays.
Thanks,
pq
> > > Le 12/02/2016 00:25, Bryce Harrington a écrit :
> > > > weston allows both short and long style options to take arguments. In
> > > > the case of short options, allow an optional space between the option
> > > > name and value. E.g., previously you could launch weston this way:
> > > >
> > > >weston -i2 -cmyconfig.ini
> > > >
> > > > now you can also launch it like this:
> > > >
> > > >weston -i 2 -c myconfig.ini
> > > >
> > > > Signed-off-by: Bryce Harrington
> > > > ---
> > > > shared/option-parser.c | 41 ++---
> > > > 1 file changed, 38 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/shared/option-parser.c b/shared/option-parser.c
> > > > index f1cc529..d5fee8e 100644
> > > > --- a/shared/option-parser.c
> > > > +++ b/shared/option-parser.c
> > > > @@ -98,14 +98,37 @@ short_option(const struct weston_option *options,
> > > > int count, char *arg)
> > > >
> > > > return 1;
> > > > }
> > > > - } else {
> > > > + } else if (arg[2]) {
> > > > return handle_option(options + k, arg + 2);
> > > > + } else {
> > > > + return 0;
> > > > }
> > > > }
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > +static int
> > > > +short_option_with_arg(const struct weston_option *options, int count,
> > > > char *arg, char *param)
> > > > +{
> > > > + int k;
> > > > +
> > > > + if (!arg[1])
> > > > + return 0;
> > > > +
> > > > + for (k = 0; k < count; k++) {
> > > > + if (options[k].short_name != arg[1])
> > > > + continue;
> > > > +
> > > > + if (options[k].type == WESTON_OPTION_BOOLEAN)
> > > > + continue;
> > > > +
> > > > + return handle_option(options + k, param);
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > int
> > > > parse_options(const struct weston_option *options,
> > > > int count, int *argc, char *argv[])
> > > > @@ -115,10 +138,22 @@ parse_options(const struct weston_option *options,
> > > > for (i = 1, j = 1; i < *argc; i++) {
> > > > if (argv[i][0] == '
Re: [PATCH v6 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests
Hi, - Original Message - > On Thu, Apr 14, 2016 at 1:56 AM, Jonas Ådahl wrote: > > > > > This makes it sound like the compositor will have to obay the limits. > > Did we end up with requiring that a correctly working compositor never > > tries to configure a size that is not within the set bounds? I think it > > should be cleared out anyhow, what the client can rely upon here anyhow. > > > > No the compositor is not required to obey the limits! This whole discussion > was based on examples of tiled window management and complaints about the > calculator being ugly due to being scaled, compared to X11 where the tiled > window manager requested sizes outside the size range, and the client > obeyed it. [...] > If the compositor cannot request a larger size then all these patches are > entirely pointless!!! Seems everyone has a different idea and a consensus might be hardly achievable here. Thing is, the client still have the option of not ack'ing the configure event, so the final word is (and remains) with the client, that's it. Considering this, not sure we need to rephrase the description once again. Cheers, Olivier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
