[Spice-devel] [PATCH linux vdagent 06/10] Make clearer distinctions between output ids

Lukáš Hrázký lhrazky at redhat.com
Mon Dec 17 17:09:43 UTC 2018


On Thu, 2018-12-13 at 16:46 -0600, Jonathon Jongsma wrote:
> There are basically three ways to refer to an output within vdagent:
>   - The index of the array of MonitorConfig message. This is essentially
>     a "spice display id"
>   - the index of the array of xrandr outputs. This is the "output index"
>   - the xrandr output id. This is the "output ID"
> 
> Previously, the "spice display id" and the "output index" were treated
> as synonymous. But in order to support more complciated setups with
> multiple display devices, etc, we need to differentiate these ideas more
> clearly.
> ---
>  src/vdagent/x11-randr.c | 50 ++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> index 2368e17..925fea8 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -187,14 +187,14 @@ find_mode_by_name (struct vdagent_x11 *x11, char *name)
>  }
>  
>  static XRRModeInfo *
> -find_mode_by_size (struct vdagent_x11 *x11, int output, int width, int height)
> +find_mode_by_size (struct vdagent_x11 *x11, int output_index, int width, int height)
>  {
>      int        	m;
>      XRRModeInfo        *ret = NULL;
>  
> -    for (m = 0; m < x11->randr.outputs[output]->nmode; m++) {
> +    for (m = 0; m < x11->randr.outputs[output_index]->nmode; m++) {
>          XRRModeInfo *mode = mode_from_id(x11,
> -                                         x11->randr.outputs[output]->modes[m]);
> +                                         x11->randr.outputs[output_index]->modes[m]);
>          if (mode && mode->width == width && mode->height == height) {
>              ret = mode;
>              break;
> @@ -341,17 +341,17 @@ static XRRModeInfo *create_new_mode(struct vdagent_x11 *x11, int output_index,
>      return find_mode_by_name(x11, modename);
>  }
>  
> -static int xrandr_add_and_set(struct vdagent_x11 *x11, int output, int x, int y,
> +static int xrandr_add_and_set(struct vdagent_x11 *x11, int output_index, int x, int y,
>                                int width, int height)
>  {
>      XRRModeInfo *mode;
>      int xid;
>      Status s;
>      RROutput outputs[1];
> -    int old_width  = x11->randr.monitor_sizes[output].width;
> -    int old_height = x11->randr.monitor_sizes[output].height;
> +    int old_width  = x11->randr.monitor_sizes[output_index].width;
> +    int old_height = x11->randr.monitor_sizes[output_index].height;
>  
> -    if (!x11->randr.res || output >= x11->randr.res->noutput || output < 0) {
> +    if (!x11->randr.res || output_index >= x11->randr.res->noutput || output_index < 0) {
>          syslog(LOG_ERR, "%s: program error: missing RANDR or bad output",
>                 __FUNCTION__);
>          return 0;
> @@ -360,21 +360,21 @@ static int xrandr_add_and_set(struct vdagent_x11 *x11, int output, int x, int y,
>          /* fail, set_best_mode will find something close. */
>          return 0;
>      }
> -    xid = x11->randr.res->outputs[output];
> -    mode = find_mode_by_size(x11, output, width, height);
> +    xid = x11->randr.res->outputs[output_index];
> +    mode = find_mode_by_size(x11, output_index, width, height);
>      if (!mode) {
> -        mode = create_new_mode(x11, output, width, height);
> +        mode = create_new_mode(x11, output_index, width, height);
>      }
>      if (!mode) {
>          syslog(LOG_ERR, "failed to add a new mode");
>          return 0;
>      }
>      XRRAddOutputMode(x11->display, xid, mode->id);
> -    x11->randr.monitor_sizes[output].width = width;
> -    x11->randr.monitor_sizes[output].height = height;
> +    x11->randr.monitor_sizes[output_index].width = width;
> +    x11->randr.monitor_sizes[output_index].height = height;
>      outputs[0] = xid;
>      vdagent_x11_set_error_handler(x11, ignore_error_handler);
> -    s = XRRSetCrtcConfig(x11->display, x11->randr.res, x11->randr.res->crtcs[output],
> +    s = XRRSetCrtcConfig(x11->display, x11->randr.res, x11->randr.res->crtcs[output_index],
>                           CurrentTime, x, y, mode->id, RR_Rotate_0, outputs,
>                           1);
>      if (vdagent_x11_restore_error_handler(x11) || (s != RRSetConfigSuccess)) {
> @@ -385,24 +385,24 @@ static int xrandr_add_and_set(struct vdagent_x11 *x11, int output, int x, int y,
>  
>      /* clean the previous name, if any */
>      if (width != old_width || height != old_height)
> -        delete_mode(x11, output, old_width, old_height);
> +        delete_mode(x11, output_index, old_width, old_height);
>  
>      return 1;
>  }
>  
> -static void xrandr_disable_output(struct vdagent_x11 *x11, int output)
> +static void xrandr_disable_nth_output(struct vdagent_x11 *x11, int output_index)
>  {
>      Status s;
>  
> -    if (!x11->randr.res || output >= x11->randr.res->noutput || output < 0) {
> +    if (!x11->randr.res || output_index >= x11->randr.res->noutput || output_index < 0) {
>          syslog(LOG_ERR, "%s: program error: missing RANDR or bad output",
>                 __FUNCTION__);
>          return;
>      }
>  
> -    XRROutputInfo *oinfo = x11->randr.outputs[output];
> +    XRROutputInfo *oinfo = x11->randr.outputs[output_index];
>      if (oinfo->ncrtc == 0) {
> -        syslog(LOG_WARNING, "Output index %i doesn't have any associated CRTCs", output);
> +        syslog(LOG_WARNING, "Output index %i doesn't have any associated CRTCs", output_index);
>          return;
>      }
>  
> @@ -415,10 +415,10 @@ static void xrandr_disable_output(struct vdagent_x11 *x11, int output)
>      if (s != RRSetConfigSuccess)
>          syslog(LOG_ERR, "failed to disable monitor");
>  
> -    delete_mode(x11, output, x11->randr.monitor_sizes[output].width,
> -                             x11->randr.monitor_sizes[output].height);
> -    x11->randr.monitor_sizes[output].width  = 0;
> -    x11->randr.monitor_sizes[output].height = 0;
> +    delete_mode(x11, output_index, x11->randr.monitor_sizes[output_index].width,
> +                             x11->randr.monitor_sizes[output_index].height);
> +    x11->randr.monitor_sizes[output_index].width  = 0;
> +    x11->randr.monitor_sizes[output_index].height = 0;
>  }
>  
>  static int set_screen_to_best_size(struct vdagent_x11 *x11, int width, int height,
> @@ -842,12 +842,12 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,
>      g_free(config);
>  
>      for (i = mon_config->num_of_monitors; i < x11->randr.res->noutput; i++)
> -        xrandr_disable_output(x11, i);
> +        xrandr_disable_nth_output(x11, i);
>  
>      /* First, disable disabled CRTCs... */
>      for (i = 0; i < mon_config->num_of_monitors; ++i) {
>          if (!monitor_enabled(&mon_config->monitors[i])) {
> -            xrandr_disable_output(x11, i);
> +            xrandr_disable_nth_output(x11, i);
>          }
>      }
>  
> @@ -870,7 +870,7 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,
>                  syslog(LOG_DEBUG, "Disabling monitor %d: %dx%d+%d+%d > (%d,%d)",
>                         i, width, height, x, y, primary_w, primary_h);
>  
> -            xrandr_disable_output(x11, i);
> +            xrandr_disable_nth_output(x11, i);
>          }
>      }
> 

Acked-by: Lukáš Hrázký <lhrazky at redhat.com>

Maybe explicitly mention in the commit message this patch only renames
some variables and a function.


More information about the Spice-devel mailing list