Re: [PATCH weston 09/14 v3] weston: Port X11 backend to new output handling API

2016-09-29 Thread Armin Krezović 
On 29.09.2016 12:17, Pekka Paalanen wrote:
> On Thu, 18 Aug 2016 18:42:37 +0200
> Armin Krezović  wrote:
> 
>> This is a complete port of the X11 backend that
>> uses recently added output handling API for output
>> configuration.
>>
>> - Output can be configured at runtime by passing the
>>   necessary configuration parameters, which can be
>>   filled in manually, obtained from the configuration
>>   file or obtained from the command line using
>>   previously added functionality. It is required that
>>   the scale and transform values are set using the
>>   previously added functionality.
>>
>> - Output can be created at runtime using the output
>>   API. The output creation only creates a pending
>>   output, which needs to be configured the same way as
>>   mentioned above.
>>
>> Same as before, a single output is created at runtime
>> using the default configuration or a configuration
>> parsed from the command line. The output-count
>> functionality is also preserved, which means more than
>> one output can be created initially, and more outputs can
>> be added at runtime using the output API.
>>
>> v2:
>>
>>  - Fix wet_configure_windowed_output_from_config() usage.
>>  - Call x11_output_disable() explicitly from
>>x11_output_destroy().
>>
>> v3:
>>
>>  - Remove unneeded free().
>>  - Disallow calling x11_output_configure more than once.
>>  - Remove unneeded checks for output->name == NULL as that
>>has been disallowed.
>>  - Use weston_compositor_add_pending_output().
>>  - Bump weston_x11_backend_config version to 2.
>>
>> Signed-off-by: Armin Krezović 
>> ---
>>  compositor/main.c  | 151 +-
>>  libweston/compositor-x11.c | 312 
>> +
>>  libweston/compositor-x11.h |  13 +-
>>  3 files changed, 235 insertions(+), 241 deletions(-)
> 
> Hi,
> 
> nice work!
> Reviewed-by: Pekka Paalanen 
> 
> 

Hi, thanks!

>> --- a/libweston/compositor-x11.c
>> +++ b/libweston/compositor-x11.c
>> @@ -59,6 +59,7 @@
>>  #include "pixman-renderer.h"
>>  #include "presentation-time-server-protocol.h"
>>  #include "linux-dmabuf.h"
>> +#include "windowed-output-api.h"
>>  
>>  #define DEFAULT_AXIS_STEP_DISTANCE 10
>>  
>> @@ -75,6 +76,8 @@ struct x11_backend {
>>  struct xkb_keymap   *xkb_keymap;
>>  unsigned int has_xkb;
>>  uint8_t  xkb_event_base;
>> +int  fullscreen;
>> +int  no_input;
>>  int  use_pixman;
>>  
>>  int  has_net_wm_state_fullscreen;
> 
> Insignificant nitpicks:
> - boolean flags should usually be 'bool'
> - negative flags often lead to double-negatives in conditions
>   (!no_input) so for readability would prefer positive flags instead
> 

Makes sense. But there are a lot of flags which use int instead of
bool and I'd rather avoid mixup. We can fix them all at some point
later on (use_pixman above is just one of them that was int since
its introduction).

> 
> Thanks,
> pq
> 




signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 09/14 v3] weston: Port X11 backend to new output handling API

2016-09-29 Thread Pekka Paalanen 
On Thu, 18 Aug 2016 18:42:37 +0200
Armin Krezović  wrote:

> This is a complete port of the X11 backend that
> uses recently added output handling API for output
> configuration.
> 
> - Output can be configured at runtime by passing the
>   necessary configuration parameters, which can be
>   filled in manually, obtained from the configuration
>   file or obtained from the command line using
>   previously added functionality. It is required that
>   the scale and transform values are set using the
>   previously added functionality.
> 
> - Output can be created at runtime using the output
>   API. The output creation only creates a pending
>   output, which needs to be configured the same way as
>   mentioned above.
> 
> Same as before, a single output is created at runtime
> using the default configuration or a configuration
> parsed from the command line. The output-count
> functionality is also preserved, which means more than
> one output can be created initially, and more outputs can
> be added at runtime using the output API.
> 
> v2:
> 
>  - Fix wet_configure_windowed_output_from_config() usage.
>  - Call x11_output_disable() explicitly from
>x11_output_destroy().
> 
> v3:
> 
>  - Remove unneeded free().
>  - Disallow calling x11_output_configure more than once.
>  - Remove unneeded checks for output->name == NULL as that
>has been disallowed.
>  - Use weston_compositor_add_pending_output().
>  - Bump weston_x11_backend_config version to 2.
> 
> Signed-off-by: Armin Krezović 
> ---
>  compositor/main.c  | 151 +-
>  libweston/compositor-x11.c | 312 
> +
>  libweston/compositor-x11.h |  13 +-
>  3 files changed, 235 insertions(+), 241 deletions(-)

Hi,

nice work!
Reviewed-by: Pekka Paalanen 


> --- a/libweston/compositor-x11.c
> +++ b/libweston/compositor-x11.c
> @@ -59,6 +59,7 @@
>  #include "pixman-renderer.h"
>  #include "presentation-time-server-protocol.h"
>  #include "linux-dmabuf.h"
> +#include "windowed-output-api.h"
>  
>  #define DEFAULT_AXIS_STEP_DISTANCE 10
>  
> @@ -75,6 +76,8 @@ struct x11_backend {
>   struct xkb_keymap   *xkb_keymap;
>   unsigned int has_xkb;
>   uint8_t  xkb_event_base;
> + int  fullscreen;
> + int  no_input;
>   int  use_pixman;
>  
>   int  has_net_wm_state_fullscreen;

Insignificant nitpicks:
- boolean flags should usually be 'bool'
- negative flags often lead to double-negatives in conditions
  (!no_input) so for readability would prefer positive flags instead


Thanks,
pq


pgp1qS4tDP337.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston 09/14 v3] weston: Port X11 backend to new output handling API

2016-08-18 Thread Armin Krezović 
This is a complete port of the X11 backend that
uses recently added output handling API for output
configuration.

- Output can be configured at runtime by passing the
  necessary configuration parameters, which can be
  filled in manually, obtained from the configuration
  file or obtained from the command line using
  previously added functionality. It is required that
  the scale and transform values are set using the
  previously added functionality.

- Output can be created at runtime using the output
  API. The output creation only creates a pending
  output, which needs to be configured the same way as
  mentioned above.

Same as before, a single output is created at runtime
using the default configuration or a configuration
parsed from the command line. The output-count
functionality is also preserved, which means more than
one output can be created initially, and more outputs can
be added at runtime using the output API.

v2:

 - Fix wet_configure_windowed_output_from_config() usage.
 - Call x11_output_disable() explicitly from
   x11_output_destroy().

v3:

 - Remove unneeded free().
 - Disallow calling x11_output_configure more than once.
 - Remove unneeded checks for output->name == NULL as that
   has been disallowed.
 - Use weston_compositor_add_pending_output().
 - Bump weston_x11_backend_config version to 2.

Signed-off-by: Armin Krezović 
---
 compositor/main.c  | 151 +-
 libweston/compositor-x11.c | 312 +
 libweston/compositor-x11.h |  13 +-
 3 files changed, 235 insertions(+), 241 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index d2df568..f782294 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1426,48 +1426,43 @@ out:
return ret;
 }
 
-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].width = output_config->width;
-   config->outputs[config->num_outputs].height = output_config->height;
-   config->outputs[config->num_outputs].transform = 
output_config->transform;
-   config->outputs[config->num_outputs].scale = output_config->scale;
-   config->outputs[config->num_outputs].name = strdup(output_config->name);
-   config->num_outputs++;
+static void
+x11_backend_output_configure(struct wl_listener *listener, void *data)
+{
+   struct weston_output *output = data;
+   struct wet_output_config defaults = {
+   .width = 1024,
+   .height = 600,
+   .scale = 1,
+   .transform = WL_OUTPUT_TRANSFORM_NORMAL
+   };
 
-   return 0;
+   if (wet_configure_windowed_output_from_config(output, ) < 0)
+   weston_log("Cannot configure output \"%s\".\n", output->name);
 }
 
 static int
 load_x11_backend(struct weston_compositor *c,
 int *argc, char **argv, struct weston_config *wc)
 {
-   struct weston_x11_backend_output_config default_output;
+   char *default_output;
+   const struct weston_windowed_output_api *api;
struct weston_x11_backend_config config = {{ 0, }};
struct weston_config_section *section;
int ret = 0;
-   int option_width = 0;
-   int option_height = 0;
-   int option_scale = 0;
int option_count = 1;
int output_count = 0;
char const *section_name;
int i;
-   uint32_t j;
+
+   struct wet_output_config *parsed_options = wet_init_parsed_options(c);
+   if (!parsed_options)
+   return -1;
 
const struct weston_option options[] = {
-  { WESTON_OPTION_INTEGER, "width", 0, _width },
-  { WESTON_OPTION_INTEGER, "height", 0, _height },
-  { WESTON_OPTION_INTEGER, "scale", 0, _scale },
+  { WESTON_OPTION_INTEGER, "width", 0, _options->width },
+  { WESTON_OPTION_INTEGER, "height", 0, _options->height },
+  { WESTON_OPTION_INTEGER, "scale", 0, _options->scale },
   { WESTON_OPTION_BOOLEAN, "fullscreen", 'f',  },
   { WESTON_OPTION_INTEGER, "output-count", 0, _count },
   { WESTON_OPTION_BOOLEAN, "no-input", 0, _input },
@@ -1476,94 +1471,66 @@ load_x11_backend(struct weston_compositor *c,
 
parse_options(options, ARRAY_LENGTH(options), argc, argv);
 
+   config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
+   config.base.struct_size = sizeof(struct weston_x11_backend_config);
+
+   /* load