weston: Add config option to enable pixman-based rendering

[Thomas Zimmermann]

Hi,

thank you for the review. The MR is pending at [1]. but it's basically
the same code as posted here. Please see my comments below.

Am 18.09.2018 um 14:43 schrieb Emmanuel Gil Peyrot:
>> diff --git a/compositor/main.c b/compositor/main.c
>> index 1e827884..e7ac52ca 100644
>> --- a/compositor/main.c
>> +++ b/compositor/main.c
>> @@ -1104,18 +1104,26 @@ load_drm_backend(struct weston_compositor *c,
>>  	struct weston_config_section *section;
>>  	struct wet_compositor *wet = to_wet_compositor(c);
>>  	int ret = 0;
>> +	int use_pixman_config_ = 0;
>> +	int32_t use_pixman_ = 0;
> 
> Weston’s coding style doesn’t use underscore suffixes anywhere, and
> these two don’t seem to add any semantics.

This code uses the same style as load_wayland_backend(). I don't mind
removing the underscore; just saying.

>>  
>> +	section = weston_config_get_section(wc, "core", NULL, NULL);
>> +	weston_config_section_get_bool(section, "use-pixman", &use_pixman_config_,
>> +				       use_pixman_config_);
>> +	use_pixman_ = use_pixman_config_;
> 
> Here you can use `false` as the default value, and remove the
> `use_pixman_config_` variable altogether.
> 
>>  
>>  	parse_options(options, ARRAY_LENGTH(options), argc, argv);
>> +	config.use_pixman = use_pixman_;
> 
> Actually you can even remove both local variables and use
> `config.use_pixman` everywhere, also make it a bool.

That's not possible. weston_config_section_get_bool() expects a pointer
to an `int` as its third argument. parse_options() expects a pointer to
an `int32_t` for WESTON_OPTION_BOOLEAN.

Both should certainly be replaced by a `bool` or at least harmonized,
but that seems worth a separate patch set.

Best regards
Thomas

[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/21

>>  
>>  	section = weston_config_get_section(wc, "core", NULL, NULL);
>>  	weston_config_section_get_string(section,
> [snip]
> 
> Same for the other parts of this file.
> 
>> diff --git a/man/weston.ini.man b/man/weston.ini.man
>> index f237fd60..cc1cb409 100644
>> --- a/man/weston.ini.man
>> +++ b/man/weston.ini.man
>> @@ -189,6 +189,12 @@ useful for debugging a crash on start-up when it would be inconvenient to
>>  launch weston directly from a debugger. Boolean, defaults to
>>  .BR false .
>>  There is also a command line option to do the same.
>> +.TP 7
>> +.BI "use-pixman=" true
>> +Enables pixman-based rendering for all outputs on backends that support it.
>> +Boolean, defaults to
>> +.BR false .
>> +There is also a command line option to do the same.
>>  
>>  .SH "LIBINPUT SECTION"
>>  The
>> -- 
>> 2.14.4
> 
> Otherwise, LGTM!
> 
> Please open a merge request on Freedesktop’s GitLab[1] instead for the
> v2 of these patches, the mailing list isn’t used for patches or reviews
> anymore.
> 
> [1] https://gitlab.freedesktop.org/wayland/weston
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180921/66fcfc65/attachment.sig>