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

[Bryce Harrington]

On Sat, Apr 30, 2016 at 11:54:15AM +0300, Giulio Camuffo wrote:
> 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.

Ah, I neglected to test that.  I'm not sure about putting option_count
to 0 but I'll take another look at the logic.

> 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.

Indeed, I also got confused by it initially.  Perhaps a different
variable name would be better to eliminate ambiguity.

Bryce