On Wed, Feb 16, 2011 at 10:59:22PM -0800, Keith Packard wrote: > This permits clients to perform incremental configuration changes > instead of requiring a complete new configuration to be written. > > Signed-off-by: Keith Packard <kei...@keithp.com> > --- > randr/rrcrtc.c | 332 > ++++++++++++++++++++++++++++++++++---------------------- > 1 files changed, 202 insertions(+), 130 deletions(-) > > diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c > index 5fe6900..07e9019 100644 > --- a/randr/rrcrtc.c > +++ b/randr/rrcrtc.c > @@ -1483,63 +1483,82 @@ RRConvertCrtcConfig(ClientPtr client, ScreenPtr > screen, > PixmapPtr pixmap; > int rc, i, j; > Rotation rotation; > + int numOutputs; > > VERIFY_RR_CRTC(x->crtc, crtc, DixSetAttrAccess); > > - if (x->mode == None) > - { > - mode = NULL; > - if (x->nOutput > 0) > - return BadMatch; > + mode = crtc->mode; > + numOutputs = crtc->numOutputs; > + > + if (x->set & RR_SetCrtcOutputs) > + numOutputs = x->nOutput; > + > + if (x->set & RR_SetCrtcMode) { > + if (x->mode == None) > + mode = NULL; > + else > + { > + VERIFY_RR_MODE(x->mode, mode, DixSetAttrAccess); > + if (x->nOutput == 0) > + return BadMatch; > + }
randrproto.txt says nothing about SetCrtcMode. > } > - else > + > + if (numOutputs) > { > - VERIFY_RR_MODE(x->mode, mode, DixSetAttrAccess); > - if (x->nOutput == 0) > + if (mode == NULL) > return BadMatch; > - } > - if (x->nOutput) > - { > - outputs = malloc(x->nOutput * sizeof (RROutputPtr)); > + outputs = malloc(numOutputs * sizeof (RROutputPtr)); > if (!outputs) > return BadAlloc; > } > - else > + else { > + if (mode != NULL) > + return BadMatch; randrproto.txt says nothing about SetCrtcOutputs. The interplay encoded here probably ought to be described in the protocol doc. > outputs = NULL; > - > - if (x->pixmap == None) > - pixmap = NULL; > - else if (x->pixmap == RR_CurrentScanoutPixmap) > - pixmap = crtc->scanoutPixmap; > - else > - { > - rc = dixLookupResourceByType((pointer *) &pixmap, x->pixmap, > - RT_PIXMAP, client, DixWriteAccess); > - if (rc != Success) { > - free(outputs); > - return rc; > - } > - /* XXX check to make sure this is a scanout pixmap */ > } > > - for (i = 0; i < x->nOutput; i++) > - { > - rc = dixLookupResourceByType((pointer *)(outputs + i), outputIds[i], > - RROutputType, client, DixSetAttrAccess); > - if (rc != Success) > + if (x->set & RR_SetCrtcPixmap) { > + if (x->pixmap == None) > + pixmap = NULL; > + else > { > - free(outputs); > - return rc; > + rc = dixLookupResourceByType((pointer *) &pixmap, x->pixmap, > + RT_PIXMAP, client, DixWriteAccess); > + if (rc != Success) { > + free(outputs); > + return rc; > + } > + /* XXX check to make sure this is a scanout pixmap */ > } > - /* validate crtc for this output */ > - for (j = 0; j < outputs[i]->numCrtcs; j++) > - if (outputs[i]->crtcs[j] == crtc) > - break; > - if (j == outputs[i]->numCrtcs) > + } else > + pixmap = crtc->scanoutPixmap; > + > + if (x->set & RR_SetCrtcOutputs) { > + for (i = 0; i < numOutputs; i++) > { > - free(outputs); > - return BadMatch; > + rc = dixLookupResourceByType((pointer *)(outputs + i), > outputIds[i], > + RROutputType, client, > DixSetAttrAccess); > + if (rc != Success) > + { > + free(outputs); > + return rc; > + } > + /* validate crtc for this output */ > + for (j = 0; j < outputs[i]->numCrtcs; j++) > + if (outputs[i]->crtcs[j] == crtc) > + break; > + if (j == outputs[i]->numCrtcs) > + { > + free(outputs); > + return BadMatch; > + } > } > + } else > + memcpy(outputs, crtc->outputs, numOutputs * sizeof (RROutputPtr)); > + > + for (i = 0; i < numOutputs; i++) > + { > /* validate mode for this output */ > for (j = 0; j < outputs[i]->numModes + outputs[i]->numUserModes; j++) > { > @@ -1555,10 +1574,11 @@ RRConvertCrtcConfig(ClientPtr client, ScreenPtr > screen, > return BadMatch; > } > } > + > /* validate clones */ > - for (i = 0; i < x->nOutput; i++) > + for (i = 0; i < numOutputs; i++) > { > - for (j = 0; j < x->nOutput; j++) > + for (j = 0; j < numOutputs; j++) > { > int k; > if (i == j) > @@ -1579,46 +1599,68 @@ RRConvertCrtcConfig(ClientPtr client, ScreenPtr > screen, > if (crtc->pScreen != screen) > return BadMatch; > > + rotation = crtc->rotation; > + if (x->set & RR_SetCrtcRotation) { > + /* > + * Validate requested rotation > + */ > + rotation = (Rotation) x->rotation; > + > + /* test the rotation bits only! */ > + switch (rotation & 0xf) { > + case RR_Rotate_0: > + case RR_Rotate_90: > + case RR_Rotate_180: > + case RR_Rotate_270: > + break; > + default: > + /* > + * Invalid rotation > + */ > + client->errorValue = x->rotation; > + free(outputs); > + return BadValue; > + } > + } > + > scr_priv = rrGetScrPriv(screen); > > config->crtc = crtc; > - config->x = x->x; > - config->y = x->y; > + config->x = crtc->x; > + config->y = crtc->y; > + if (x->set & RR_SetCrtcPosition) { > + config->x = x->x; > + config->y = x->y; > + } > config->mode = mode; > - config->rotation = x->rotation; > - config->numOutputs = x->nOutput; > + config->rotation = rotation; > + config->numOutputs = numOutputs; > config->outputs = outputs; > - PictTransform_from_xRenderTransform(&config->sprite_position_transform, > - &x->spritePositionTransform); > - PictTransform_from_xRenderTransform(&config->sprite_image_transform, > - &x->spriteImageTransform); > - > pixman_f_transform_from_pixman_transform(&config->sprite_position_f_transform, > - > &config->sprite_position_transform); > - > pixman_f_transform_from_pixman_transform(&config->sprite_image_f_transform, > - &config->sprite_image_transform); > - config->pixmap = pixmap; > - config->pixmap_x = x->xPixmap; > - config->pixmap_y = x->yPixmap; > - > - /* > - * Validate requested rotation > - */ > - rotation = (Rotation) x->rotation; > > - /* test the rotation bits only! */ > - switch (rotation & 0xf) { > - case RR_Rotate_0: > - case RR_Rotate_90: > - case RR_Rotate_180: > - case RR_Rotate_270: > - break; > - default: > - /* > - * Invalid rotation > - */ > - client->errorValue = x->rotation; > - free(outputs); > - return BadValue; > + if (x->set & RR_SetCrtcSpritePositionTransform) { > + > PictTransform_from_xRenderTransform(&config->sprite_position_transform, > + &x->spritePositionTransform); > + > pixman_f_transform_from_pixman_transform(&config->sprite_position_f_transform, > + > &config->sprite_position_transform); > + } else { > + config->sprite_position_transform = > crtc->client_sprite_position_transform; > + config->sprite_position_f_transform = > crtc->client_sprite_f_position_transform; > + } > + if (x->set & RR_SetCrtcSpriteImageTransform) { > + PictTransform_from_xRenderTransform(&config->sprite_image_transform, > + &x->spriteImageTransform); > + > pixman_f_transform_from_pixman_transform(&config->sprite_image_f_transform, > + > &config->sprite_image_transform); > + } else { > + config->sprite_image_transform = crtc->client_sprite_image_transform; > + config->sprite_image_f_transform = > crtc->client_sprite_f_image_transform; > + } > + config->pixmap = pixmap; > + config->pixmap_x = crtc->x; > + config->pixmap_y = crtc->y; > + if (x->set & RR_SetCrtcPixmapPosition) { > + config->pixmap_x = x->xPixmap; > + config->pixmap_y = x->yPixmap; > } > > if (mode) > @@ -1628,7 +1670,7 @@ RRConvertCrtcConfig(ClientPtr client, ScreenPtr screen, > /* > * requested rotation or reflection not supported by screen > */ > - client->errorValue = x->rotation; > + client->errorValue = rotation; > free(outputs); > return BadMatch; > } > @@ -1639,13 +1681,13 @@ RRConvertCrtcConfig(ClientPtr client, ScreenPtr > screen, > */ > if (pixmap) > { > - if (x->xPixmap + mode->mode.width > pixmap->drawable.width) { > - client->errorValue = x->xPixmap; > + if (config->pixmap_x + mode->mode.width > pixmap->drawable.width) > { > + client->errorValue = config->pixmap_x; > free(outputs); > return BadValue; > } > - if (x->yPixmap + mode->mode.height > pixmap->drawable.height) { > - client->errorValue = x->yPixmap; > + if (config->pixmap_y + mode->mode.height > > pixmap->drawable.height) { > + client->errorValue = config->pixmap_y; > free(outputs); > return BadValue; > } > @@ -1687,13 +1729,15 @@ ProcRRSetCrtcConfigs (ClientPtr client) > int num_configs = 0; > int rc, i; > int extra_len; > - int num_output_ids; > + int num_output_ids = 0; > > REQUEST_AT_LEAST_SIZE(xRRSetCrtcConfigsReq); > > extra_len = client->req_len - > bytes_to_int32(sizeof(xRRSetCrtcConfigsReq)); > > - num_configs = stuff->nConfigs; > + num_configs = 0; > + if (stuff->set & RR_SetScreenCrtcs) > + num_configs = stuff->nConfigs; > > /* Check request length against number of configs specified */ > if (num_configs * (sizeof (xRRCrtcConfig) >> 2) > extra_len) > @@ -1702,10 +1746,11 @@ ProcRRSetCrtcConfigs (ClientPtr client) > extra_len -= num_configs * (sizeof (xRRCrtcConfig) >> 2); > x_configs = (xRRCrtcConfig *) (stuff + 1); > > - /* Check remaining request length against number of outputs */ > - num_output_ids = 0; > - for (i = 0; i < num_configs; i++) > - num_output_ids += x_configs[i].nOutput; > + if (stuff->set & RR_SetScreenCrtcs) { > + /* Check remaining request length against number of outputs */ > + for (i = 0; i < num_configs; i++) > + num_output_ids += x_configs[i].nOutput; > + } > > if (extra_len != num_output_ids) > return BadLength; > @@ -1724,57 +1769,84 @@ ProcRRSetCrtcConfigs (ClientPtr client) > goto sendReply; > } > > - if (stuff->widthInMillimeters == 0 || stuff->heightInMillimeters == 0) > - { > - client->errorValue = 0; > - return BadValue; > + if (stuff->set & RR_SetScreenSizeInMillimeters) { > + if (stuff->widthInMillimeters == 0 || stuff->heightInMillimeters == 0) > + { > + client->errorValue = 0; > + return BadValue; > + } > + screen_config.mm_width = stuff->widthInMillimeters; > + screen_config.mm_height = stuff->heightInMillimeters; > + } else { > + screen_config.mm_width = screen->mmWidth; > + screen_config.mm_height = screen->mmHeight; > } > > - if (stuff->screenPixmapWidth < scr_priv->minWidth || > - scr_priv->maxWidth < stuff->screenPixmapWidth) > - { > - client->errorValue = stuff->screenPixmapWidth; > - return BadValue; > - } > - if (stuff->screenPixmapHeight < scr_priv->minHeight || > - scr_priv->maxHeight < stuff->screenPixmapHeight) > - { > - client->errorValue = stuff->screenPixmapHeight; > - return BadValue; > + if (stuff->set & RR_SetScreenPixmapSize) { > + if (stuff->screenPixmapWidth < scr_priv->minWidth || > + scr_priv->maxWidth < stuff->screenPixmapWidth) > + { > + client->errorValue = stuff->screenPixmapWidth; > + return BadValue; > + } > + if (stuff->screenPixmapHeight < scr_priv->minHeight || > + scr_priv->maxHeight < stuff->screenPixmapHeight) > + { > + client->errorValue = stuff->screenPixmapHeight; > + return BadValue; > + } > + screen_config.screen_pixmap_width = stuff->screenPixmapWidth; > + screen_config.screen_pixmap_height = stuff->screenPixmapHeight; > + } else { > + PixmapPtr screen_pixmap = screen->GetScreenPixmap(screen); > + screen_config.screen_pixmap_width = screen_pixmap->drawable.width; > + screen_config.screen_pixmap_height = screen_pixmap->drawable.height; > + } > + > + if (stuff->set & RR_SetScreenSize) { > + if (stuff->screenWidth <= 0 || stuff->screenWidth > 32767) { > + client->errorValue = stuff->screenWidth; > + return BadValue; > + } > + screen_config.screen_width = stuff->screenWidth; > + > + if (stuff->screenHeight <= 0 || stuff->screenHeight > 32767) { > + client->errorValue = stuff->screenHeight; > + return BadValue; > + } > + screen_config.screen_height = stuff->screenHeight; > + } else { > + WindowPtr root = screen->root; > + screen_config.screen_width = root->drawable.width; > + screen_config.screen_height = root->drawable.height; > } > > - screen_config.screen_pixmap_width = stuff->screenPixmapWidth; > - screen_config.screen_pixmap_height = stuff->screenPixmapHeight; > - screen_config.screen_width = stuff->screenWidth; > - screen_config.screen_height = stuff->screenHeight; > - screen_config.mm_width = stuff->widthInMillimeters; > - screen_config.mm_height = stuff->heightInMillimeters; > + if (num_configs > 0) { Existing bug, but it's not clear from randrproto.txt that RRSetCrtcConfigs doesn't do anything if 'set' doesn't contain SetScreenCrtcs. From the wording of the protocol doc, I would expect set = RR_SetScreenPixmapSize | RR_SetScreenSizeInMillimeters | RR_SetScreenSize to change those three things. > > - output_ids = (RROutput *) (x_configs + num_configs); > + output_ids = (RROutput *) (x_configs + num_configs); > + /* > + * Convert protocol crtc configurations into > + * server crtc configurations > + */ > + configs = calloc(num_configs, sizeof (RRCrtcConfigRec)); > + if (configs == NULL) > + return BadAlloc; > + for (i = 0; i < num_configs; i++) { > + rc = RRConvertCrtcConfig(client, screen, &screen_config, > + &configs[i], > + &x_configs[i], output_ids); > + if (rc != Success) { > + rep.status = RRSetConfigFailed; > + goto sendReply; > + } > + output_ids += x_configs[i].nOutput; > + } > > - /* > - * Convert protocol crtc configurations into > - * server crtc configurations > - */ > - configs = calloc(num_configs, sizeof (RRCrtcConfigRec)); > - if (num_configs > 0 && configs == NULL) > - return BadAlloc; > - for (i = 0; i < num_configs; i++) { > - rc = RRConvertCrtcConfig(client, screen, &screen_config, > - &configs[i], > - &x_configs[i], output_ids); > - if (rc != Success) { > + if (!RRSetCrtcConfigs (screen, &screen_config, configs, num_configs)) > + { > rep.status = RRSetConfigFailed; > goto sendReply; > } > - output_ids += x_configs[i].nOutput; > - } > - > - if (num_configs && > - !RRSetCrtcConfigs (screen, &screen_config, configs, num_configs)) > - { > - rep.status = RRSetConfigFailed; > - goto sendReply; > } > rep.status = RRSetConfigSuccess; > scr_priv->lastSetTime = currentTime; > -- > 1.7.2.3 > Other than the protocol doc comments and the existing bug noted above, Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> However, I do need to object strongly to such a major protocol change being made after RC2 of a release cycle. We're supposed to be making only critical bug fixes at this point! Planning fail. -- Aaron _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel