[PATCH weston v6 06/12] x11: port the x11 backend to the new init api

[Giulio Camuffo]

2016-04-30 0:38 GMT+03:00 Bryce Harrington <bryce at osg.samsung.com>:
> On Sun, Apr 17, 2016 at 05:31:32PM +0300, Giulio Camuffo wrote:
>> Sorry, i keep seeing things after i hit send...
>>
>> 2016-04-17 17:10 GMT+03:00 Giulio Camuffo <giuliocamuffo at gmail.com>:
>> > 2016-04-16 6:28 GMT+03:00 Bryce Harrington <bryce at osg.samsung.com>:
>> >> +static int
>> >> +load_x11_backend(struct weston_compositor *c, char const * backend,
>> >> +                int *argc, char **argv, struct weston_config *wc)
>> >> +{
>> >> +       struct weston_x11_backend_output_config default_output;
>> >> +       struct weston_x11_backend_config *config;
>> >> +       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;
>> >> +
>> >> +       config = zalloc(sizeof(struct weston_x11_backend_config));
>> >> +       if (config == NULL)
>> >> +               return -1;
>> >> +
>> >> +       const struct weston_option options[] = {
>> >> +              { WESTON_OPTION_INTEGER, "width", 0, &option_width },
>> >> +              { WESTON_OPTION_INTEGER, "height", 0, &option_height },
>> >> +              { WESTON_OPTION_INTEGER, "scale", 0, &option_scale },
>> >> +              { WESTON_OPTION_BOOLEAN, "fullscreen", 'f', &config->fullscreen },
>> >> +              { WESTON_OPTION_INTEGER, "output-count", 0, &option_count },
>> >> +              { WESTON_OPTION_BOOLEAN, "no-input", 0, &config->no_input },
>> >> +              { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
>> >> +       };
>> >> +
>> >> +       parse_options(options, ARRAY_LENGTH(options), argc, argv);
>> >> +
>> >> +       section = NULL;
>> >> +       while (weston_config_next_section(wc, &section, &section_name)) {
>> >> +               struct weston_x11_backend_output_config current_output = { 0, };
>> >> +               char *t;
>> >> +               char *mode;
>> >> +
>> >> +               if (strcmp(section_name, "output") != 0) {
>> >> +                       continue;
>> >> +               }
>> >> +
>> >> +               weston_config_section_get_string(section, "name", &current_output.name, NULL);
>> >> +               if (current_output.name == NULL || current_output.name[0] != 'X') {
>> >> +                       free(current_output.name);
>> >> +                       continue;
>> >> +               }
>> >> +
>> >> +               weston_config_section_get_string(section, "mode", &mode, "1024x600");
>> >> +               if (sscanf(mode, "%dx%d", &current_output.width,
>> >> +                          &current_output.height) != 2) {
>> >> +                       weston_log("Invalid mode \"%s\" for output %s\n",
>> >> +                                  mode, current_output.name);
>> >> +                       current_output.width = 1024;
>> >> +                       current_output.height = 600;
>> >> +               }
>> >> +               free(mode);
>> >> +               if (current_output.width < 1)
>> >> +                       current_output.width = 1024;
>> >> +               if (current_output.height < 1)
>> >> +                       current_output.height = 600;
>> >> +               if (option_width)
>> >> +                       current_output.width = option_width;
>> >> +               if (option_height)
>> >> +                       current_output.height = option_height;
>> >> +
>> >> +               weston_config_section_get_int(section, "scale", &current_output.scale, 1);
>> >> +               if (option_scale)
>> >> +                       current_output.scale = option_scale;
>> >> +
>> >> +               weston_config_section_get_string(section,
>> >> +                                                "transform", &t, "normal");
>> >> +               if (weston_parse_transform(t, &current_output.transform) < 0)
>> >> +                       weston_log("Invalid transform \"%s\" for output %s\n",
>> >> +                                  t, current_output.name);
>> >> +               free(t);
>> >> +
>> >> +               if (weston_x11_backend_config_append_output_config(config, &current_output) < 0) {
>> >> +                       ret = -1;
>> >> +                       goto error;
>> >> +               }
>> >> +
>> >> +               output_count++;
>> >> +               if (option_count && output_count >= option_count)
>> >> +                       break;
>> >> +       }
>> >> +
>> >> +       default_output.name = NULL;
>> >> +       default_output.width = option_width ? option_width : 1024;
>> >> +       default_output.height = option_height ? option_height : 600;
>> >> +       default_output.scale = option_scale ? option_scale : 1;
>> >> +       default_output.transform = WL_OUTPUT_TRANSFORM_NORMAL;
>> >> +
>> >> +       for (i = output_count; i < option_count; i++) {
>>
>> This won't create any output if the --output-count is not used, that
>> is if option_count is 0. The old code used a third variable, which if
>> option_count is 0 is set to 1, otherwise to option_count. You should
>> use that in this loop, while option_count is correctly used in the
>> loop above.
>
> No, check again.  I changed option_count to be initialized as 1.  If you
> run the code you'll see it does actually work properly.  The only way
> for option_count to be 0 is if the user specifically requested
> --output_count=0, which I think we can leave as is.  If they want no
> outputs I suppose they know what they're doing (maybe useful for testing
> or something.)

I just tried. With two X outputs defined in weston.ini, without using
the --output-count option, only one output is created. That's because
option_count is 1 so "if (option_count && output_count >=
option_count)" is true at the first time, so it breaks out.
option_count should be initialized to 0, and then it works right as
far as i can see.
You are right though that my previous email was wrong. I failed to
notice that there is both option_count and output_count, they are very
easy to confuse.


Cheers,
Giulio

>
> Bryce