[PATCH] dix/glx/composite: consolidate visual resize in one place.
Peter Hutterer
peter.hutterer at who-t.net
Tue Sep 29 16:06:42 PDT 2009
On Tue, Sep 29, 2009 at 11:49:09AM +1000, Dave Airlie wrote:
> The previous code was copied and in both cases incorrectly fixed
> up the colormaps after resizing the visuals, this patch consolidates
> the visual resize + colormaps fixups in one place. This version
> also consolidates the vid allocation for the DepthPtr inside the
> function.
>
> I'm not 100% sure colormap.[ch] is the correct place for this but
> visuals are mostly created in fb and I know thats not the place to
> be resizing them.
>
> Fixes fd.o bug #19470.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> composite/compinit.c | 59 +++-------------------------------------------
> dix/colormap.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
> glx/glxscreens.c | 58 ++-------------------------------------------
> include/colormap.h | 5 ++++
> 4 files changed, 76 insertions(+), 110 deletions(-)
>
> diff --git a/composite/compinit.c b/composite/compinit.c
> index 6159e4e..96ac70f 100644
> --- a/composite/compinit.c
> +++ b/composite/compinit.c
> @@ -248,15 +248,9 @@ static Bool
> compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
> CompAlternateVisual *alt)
> {
> - VisualPtr visual, visuals;
> - int i;
> - int numVisuals;
> - XID *installedCmaps;
> - ColormapPtr installedCmap;
> - int numInstalledCmaps;
> + VisualPtr visual;
> DepthPtr depth;
> PictFormatPtr pPictFormat;
> - VisualID *vid;
> unsigned long alphaMask;
>
> /*
> @@ -277,54 +271,13 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
> pPictFormat->direct.red != pScreen->visuals[0].offsetRed)
> return FALSE;
>
> - vid = xalloc(sizeof(VisualID));
> - if (!vid)
> - return FALSE;
> -
> - /* Find the installed colormaps */
> - installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
> - if (!installedCmaps) {
> - xfree(vid);
> - return FALSE;
> - }
> - numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen,
> - installedCmaps);
> -
> - /* realloc the visual array to fit the new one in place */
> - numVisuals = pScreen->numVisuals;
> - visuals = xrealloc(pScreen->visuals, (numVisuals + 1) * sizeof(VisualRec));
> - if (!visuals) {
> - xfree(vid);
> - xfree(installedCmaps);
> - return FALSE;
> + if (ResizeVisualArray(pScreen, 1, depth) == FALSE) {
> + return FALSE;
> }
>
> - /*
> - * Fix up any existing installed colormaps -- we'll assume that
> - * the only ones created so far have been installed. If this
> - * isn't true, we'll have to walk the resource database looking
> - * for all colormaps.
> - */
> - for (i = 0; i < numInstalledCmaps; i++) {
> - int j, rc;
> -
> - rc = dixLookupResourceByType((pointer *)&installedCmap,
> - installedCmaps[i], RT_COLORMAP,
> - serverClient, DixReadAccess);
> - if (rc != Success)
> - continue;
> - j = installedCmap->pVisual - pScreen->visuals;
> - installedCmap->pVisual = &visuals[j];
> - }
> -
> - xfree(installedCmaps);
> -
> - pScreen->visuals = visuals;
> - visual = visuals + pScreen->numVisuals; /* the new one */
> - pScreen->numVisuals++;
> + visual = pScreen->visuals + (pScreen->numVisuals - 1); /* the new one */
>
> /* Initialize the visual */
> - visual->vid = FakeClientID (0);
> visual->bitsPerRGBValue = 8;
> if (PICT_FORMAT_TYPE(alt->format) == PICT_TYPE_COLOR) {
> visual->class = PseudoColor;
> @@ -357,10 +310,6 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
> /* remember the visual ID to detect auto-update windows */
> compRegisterAlternateVisuals(cs, &visual->vid, 1);
>
> - /* Fix up the depth */
> - *vid = visual->vid;
> - depth->numVids = 1;
> - depth->vids = vid;
> return TRUE;
> }
>
> diff --git a/dix/colormap.c b/dix/colormap.c
> index a5a006e..6f20c71 100644
> --- a/dix/colormap.c
> +++ b/dix/colormap.c
> @@ -2690,3 +2690,67 @@ IsMapInstalled(Colormap map, WindowPtr pWin)
> xfree(pmaps);
> return (found);
> }
> +
> +struct colormap_lookup_data {
> + ScreenPtr pScreen;
> + VisualPtr visuals;
> +};
> +
> +static void _colormap_find_resource(pointer value, XID id,
> + pointer cdata)
> +{
> + struct colormap_lookup_data *cmap_data = cdata;
> + VisualPtr visuals = cmap_data->visuals;
> + ScreenPtr pScreen = cmap_data->pScreen;
> + ColormapPtr cmap = value;
> + int j;
> +
> + j = cmap->pVisual - pScreen->visuals;
> + cmap->pVisual = &visuals[j];
> +}
> +
> +/* something has realloced the visuals, instead of breaking
> + ABI fix it up here - glx and compsite did this wrong */
> +Bool
> +ResizeVisualArray(ScreenPtr pScreen, int new_visual_count,
> + DepthPtr depth)
> +{
> + struct colormap_lookup_data cdata;
> + int numVisuals;
> + VisualPtr visuals;
> + XID *vids, vid;
> + int first_new_vid, first_new_visual, i;
> +
> + first_new_vid = depth->numVids;
> + first_new_visual = pScreen->numVisuals;
> +
> + vids = xrealloc(depth->vids, (depth->numVids + new_visual_count) * sizeof(XID));
> + if (!vids)
> + return FALSE;
> +
> + /* its realloced now no going back if we fail the next one */
> + depth->vids = vids;
> +
> + numVisuals = pScreen->numVisuals + new_visual_count;
> + visuals = xrealloc(pScreen->visuals, numVisuals * sizeof(VisualRec));
> + if (!visuals) {
> + return FALSE;
> + }
> +
> + cdata.visuals = visuals;
> + cdata.pScreen = pScreen;
> + FindClientResourcesByType(serverClient, RT_COLORMAP, _colormap_find_resource, &cdata);
> +
> + pScreen->visuals = visuals;
> +
> + for (i = 0; i < new_visual_count; i++) {
> + vid = FakeClientID(0);
> + pScreen->visuals[first_new_visual + i].vid = vid;
> + vids[first_new_vid + i] = vid;
> + }
> +
> + depth->numVids += new_visual_count;
> + pScreen->numVisuals += new_visual_count;
> +
> + return TRUE;
> +}
> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
> index 81faddd..7d29d31 100644
> --- a/glx/glxscreens.c
> +++ b/glx/glxscreens.c
> @@ -250,12 +250,8 @@ GLint glxConvertToXVisualType(int visualType)
> static VisualPtr
> AddScreenVisuals(ScreenPtr pScreen, int count, int d)
> {
> - XID *installedCmaps, *vids, vid;
> - int numInstalledCmaps, numVisuals, i, j;
> - VisualPtr visuals;
> - ColormapPtr installedCmap;
> + int i;
> DepthPtr depth;
> - int rc;
>
> depth = NULL;
> for (i = 0; i < pScreen->numDepths; i++) {
> @@ -267,56 +263,8 @@ AddScreenVisuals(ScreenPtr pScreen, int count, int d)
> if (depth == NULL)
> return NULL;
>
> - /* Find the installed colormaps */
> - installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
> - if (!installedCmaps)
> - return NULL;
> -
> - numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen, installedCmaps);
> -
> - /* realloc the visual array to fit the new one in place */
> - numVisuals = pScreen->numVisuals;
> - visuals = xrealloc(pScreen->visuals, (numVisuals + count) * sizeof(VisualRec));
> - if (!visuals) {
> - xfree(installedCmaps);
> - return NULL;
> - }
> -
> - vids = xrealloc(depth->vids, (depth->numVids + count) * sizeof(XID));
> - if (vids == NULL) {
> - xfree(installedCmaps);
> - xfree(visuals);
> - return NULL;
> - }
> -
> - /*
> - * Fix up any existing installed colormaps -- we'll assume that
> - * the only ones created so far have been installed. If this
> - * isn't true, we'll have to walk the resource database looking
> - * for all colormaps.
> - */
> - for (i = 0; i < numInstalledCmaps; i++) {
> - rc = dixLookupResourceByType((pointer *)&installedCmap,
> - installedCmaps[i], RT_COLORMAP,
> - serverClient, DixReadAccess);
> - if (rc != Success)
> - continue;
> - j = installedCmap->pVisual - pScreen->visuals;
> - installedCmap->pVisual = &visuals[j];
> - }
> -
> - xfree(installedCmaps);
> -
> - for (i = 0; i < count; i++) {
> - vid = FakeClientID(0);
> - visuals[pScreen->numVisuals + i].vid = vid;
> - vids[depth->numVids + i] = vid;
> - }
> -
> - pScreen->visuals = visuals;
> - pScreen->numVisuals += count;
> - depth->vids = vids;
> - depth->numVids += count;
> + if (ResizeVisualArray(pScreen, count, depth) == FALSE)
> + return NULL;
>
> /* Return a pointer to the first of the added visuals. */
> return pScreen->visuals + pScreen->numVisuals - count;
> diff --git a/include/colormap.h b/include/colormap.h
> index a3467c9..eb0f670 100644
> --- a/include/colormap.h
> +++ b/include/colormap.h
> @@ -179,4 +179,9 @@ extern _X_EXPORT int IsMapInstalled(
> Colormap /*map*/,
> WindowPtr /*pWin*/);
>
> +extern _X_EXPORT Bool ResizeVisualArray(
> + ScreenPtr /* pScreen */,
> + int /* new_vis_count */,
> + DepthPtr /* depth */);
> +
_X_EXPORT?
Will drivers be using this?
Cheers,
Peter
More information about the xorg-devel
mailing list