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

[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

[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