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)
signature.asc
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
