[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-xorg-devel
Subject: Re: [PATCH] hw: Fix visual colormap_size mixup causing "missing" visuals.
From: Alex Orange <crazycasta () gmail ! com>
Date: 2014-11-30 21:38:42
Message-ID: CABpR-zSC8mGncH7Ff0tgc-R5u5TOmKWY3xaM=fK8wbbz16vb-Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Please ignore the first patch, got old comment lines left in by accident.
Probably forgot to git add.
Alex
On Sun, Nov 30, 2014 at 2:37 PM, Alex Orange <crazycasta@gmail.com> wrote:
> Likely fixes: https://bugs.freedesktop.org/show_bug.cgi?id=24642
>
> dmx uses fb to handle many action. As such is uses fbScreenInit. The
> trouble is that fbScreenInit uses mi to generate the visuals. mi has
> trouble with 32-bit (rgba) depths. It tries to treat these depths as
> 10-bits per pixels. This results in colormap_sizes in the visuals being
> 2048 instead of 256. Also, the comments in micmap.c about the macros,
> _CE is the relevant one, suggest that these numbers may be driver
> defined. To this end, this patch simply copies the visuals into the
> visuals and depths structs instead of trying to coax mi into creating
> the right visuals. The code was mostly just taken from xnest with
> changes since dmx uses fb. The result depends on miScreenInit simply
> stuffing visuals in the screen and not doing anything more with them. In
> the future perhaps fbScreenInit can take an optional set of visuals to
> use instead of calling miInitVisuals.
>
> Signed-off-by: Alex Orange <crazycasta@gmail.com>
> Tested-by: Alex Orange <crazycasta@gmail.com>
> ---
> hw/dmx/dmxscrinit.c | 159
> ++++++++++++++++++++++++++++++++++++++++------------
> hw/dmx/dmxscrinit.h | 5 ++
> 2 files changed, 129 insertions(+), 35 deletions(-)
>
> diff --git a/hw/dmx/dmxscrinit.c b/hw/dmx/dmxscrinit.c
> index 963d3a9..3c68708 100644
> --- a/hw/dmx/dmxscrinit.c
> +++ b/hw/dmx/dmxscrinit.c
> @@ -54,9 +54,12 @@
>
> #include "dmxpict.h"
>
> -#include "fb.h"
> +#include <X11/X.h>
> +#include "mi.h"
> #include "mipointer.h"
> #include "micmap.h"
> +#include "resource.h"
> +#include "fb.h"
>
> extern Bool dmxCloseScreen(ScreenPtr pScreen);
> static Bool dmxSaveScreen(ScreenPtr pScreen, int what);
> @@ -172,12 +175,28 @@ dmxBEScreenInit(ScreenPtr pScreen)
> }
> }
>
> +static int
> +offset(unsigned long mask)
> +{
> + int count;
> +
> + for (count = 0; !(mask & 1) && count < 32; count++)
> + mask >>= 1;
> +
> + return count;
> +}
> +
> /** Initialize screen number \a pScreen->myNum. */
> Bool
> dmxScreenInit(ScreenPtr pScreen, int argc, char *argv[])
> {
> DMXScreenInfo *dmxScreen = &dmxScreens[pScreen->myNum];
> - int i, j;
> + int i, j, depthIndex;
> + VisualPtr visuals;
> + DepthPtr depths;
> + int numVisuals, numDepths;
> + VisualID defaultBEVisual, defaultVisual;
> + int rootDepth;
>
> if (!dixRegisterPrivateKey(&dmxScreenPrivateKeyRec, PRIVATE_SCREEN,
> 0))
> return FALSE;
> @@ -202,41 +221,95 @@ dmxScreenInit(ScreenPtr pScreen, int argc, char
> *argv[])
> if (!dmxInitPixmap(pScreen))
> return FALSE;
>
> - /*
> - * Initalise the visual types. miSetVisualTypesAndMasks() requires
> - * that all of the types for each depth be collected together. It's
> - * intended for slightly different usage to what we would like here.
> - * Maybe a miAddVisualTypeAndMask() function will be added to make
> - * things easier here.
> - */
> - for (i = 0; i < dmxScreen->beNumDepths; i++) {
> - int depth;
> - int visuals = 0;
> - int bitsPerRgb = 0;
> - int preferredClass = -1;
> - Pixel redMask = 0;
> - Pixel greenMask = 0;
> - Pixel blueMask = 0;
> -
> - depth = dmxScreen->beDepths[i];
> - for (j = 0; j < dmxScreen->beNumVisuals; j++) {
> - XVisualInfo *vi;
> -
> - vi = &dmxScreen->beVisuals[j];
> - if (vi->depth == depth) {
> - /* Assume the masks are all the same. */
> - visuals |= (1 << vi->class);
> - bitsPerRgb = vi->bits_per_rgb;
> - redMask = vi->red_mask;
> - greenMask = vi->green_mask;
> - blueMask = vi->blue_mask;
> - if (j == dmxScreen->beDefVisualIndex) {
> - preferredClass = vi->class;
> - }
> + visuals = (VisualPtr) malloc(dmxScreen->beNumVisuals *
> sizeof(VisualRec));
> + numVisuals = 0;
> +
> + depths = (DepthPtr) malloc(MAXDEPTH * sizeof(DepthRec));
> + depths[0].depth = 1;
> + depths[0].numVids = 0;
> + depths[0].vids = (VisualID *) malloc(MAXVISUALSPERDEPTH *
> sizeof(VisualID));
> + numDepths = 1;
> +
> + defaultBEVisual =
> XVisualIDFromVisual(DefaultVisual(dmxScreen->beDisplay,
> + DefaultScreen
> +
> (dmxScreen->beDisplay)));
> + rootDepth = UNDEFINED;
> + defaultVisual = UNDEFINED;
> +
> + for (i = 0; i < dmxScreen->beNumVisuals; i++) {
> + visuals[numVisuals].class = dmxScreen->beVisuals[i].class;
> + visuals[numVisuals].bitsPerRGBValue =
> + dmxScreen->beVisuals[i].bits_per_rgb;
> + visuals[numVisuals].ColormapEntries =
> + dmxScreen->beVisuals[i].colormap_size;
> + visuals[numVisuals].nplanes = dmxScreen->beVisuals[i].depth;
> + visuals[numVisuals].redMask = dmxScreen->beVisuals[i].red_mask;
> + visuals[numVisuals].greenMask =
> dmxScreen->beVisuals[i].green_mask;
> + visuals[numVisuals].blueMask = dmxScreen->beVisuals[i].blue_mask;
> + visuals[numVisuals].offsetRed =
> + offset(dmxScreen->beVisuals[i].red_mask);
> + visuals[numVisuals].offsetGreen =
> + offset(dmxScreen->beVisuals[i].green_mask);
> + visuals[numVisuals].offsetBlue =
> + offset(dmxScreen->beVisuals[i].blue_mask);
> +
> + /* Check for and remove duplicates. */
> + for (j = 0; j < numVisuals; j++) {
> + if (visuals[numVisuals].class == visuals[j].class &&
> + visuals[numVisuals].bitsPerRGBValue ==
> + visuals[j].bitsPerRGBValue &&
> + visuals[numVisuals].ColormapEntries ==
> + visuals[j].ColormapEntries &&
> + visuals[numVisuals].nplanes == visuals[j].nplanes &&
> + visuals[numVisuals].redMask == visuals[j].redMask &&
> + visuals[numVisuals].greenMask == visuals[j].greenMask &&
> + visuals[numVisuals].blueMask == visuals[j].blueMask &&
> + visuals[numVisuals].offsetRed == visuals[j].offsetRed &&
> + visuals[numVisuals].offsetGreen == visuals[j].offsetGreen
> &&
> + visuals[numVisuals].offsetBlue == visuals[j].offsetBlue)
> + break;
> + }
> + if (j < numVisuals)
> + break;
> +
> + visuals[numVisuals].vid = FakeClientID(0);
> +
> + depthIndex = UNDEFINED;
> + for (j = 0; j < numDepths; j++)
> + if (depths[j].depth == dmxScreen->beVisuals[i].depth) {
> + depthIndex = j;
> + break;
> }
> +
> + if (depthIndex == UNDEFINED) {
> + depthIndex = numDepths;
> + depths[depthIndex].depth = dmxScreen->beVisuals[i].depth;
> + depths[depthIndex].numVids = 0;
> + depths[depthIndex].vids =
> + (VisualID *) malloc(MAXVISUALSPERDEPTH *
> sizeof(VisualID));
> + numDepths++;
> }
> - miSetVisualTypesAndMasks(depth, visuals, bitsPerRgb,
> preferredClass,
> - redMask, greenMask, blueMask);
> + if (depths[depthIndex].numVids >= MAXVISUALSPERDEPTH) {
> + FatalError("Visual table overflow");
> + }
> + depths[depthIndex].vids[depths[depthIndex].numVids] =
> + visuals[numVisuals].vid;
> + depths[depthIndex].numVids++;
> +
> + if (dmxScreen->beVisuals[i].visualid == defaultBEVisual) {
> + rootDepth = visuals[numVisuals].nplanes;
> + defaultVisual = visuals[numVisuals].vid;
> + }
> +
> + numVisuals++;
> + }
> + visuals = (VisualPtr) realloc(visuals, numVisuals *
> sizeof(VisualRec));
> +
> + if (rootDepth == UNDEFINED) {
> + rootDepth = visuals[0].nplanes;
> + }
> + if (defaultVisual == UNDEFINED) {
> + defaultVisual = visuals[0].vid;
> }
>
> fbScreenInit(pScreen,
> @@ -245,6 +318,22 @@ dmxScreenInit(ScreenPtr pScreen, int argc, char
> *argv[])
> dmxScreen->scrnHeight,
> dmxScreen->beXDPI,
> dmxScreen->beXDPI, dmxScreen->scrnWidth,
> dmxScreen->beBPP);
> +
> + // fbScreenInit calls miInitVisuals which makes uncontrollable
> colormap
> + // sizes. The following overrides these visuals (pending a better
> method).
> + for (i = 0; i < pScreen->numDepths; i++) {
> + free(pScreen->allowedDepths[i].vids);
> + }
> + free(pScreen->allowedDepths);
> + free(pScreen->visuals);
> +
> + pScreen->visuals = visuals;
> + pScreen->numVisuals = numVisuals;
> + pScreen->rootVisual = defaultVisual;
> + pScreen->allowedDepths = depths;
> + pScreen->numDepths = numDepths;
> + pScreen->rootDepth = rootDepth;
> +
> (void) dmxPictureInit(pScreen, 0, 0);
>
> /* Not yet... */
> diff --git a/hw/dmx/dmxscrinit.h b/hw/dmx/dmxscrinit.h
> index 9fe9c98..23d2f62 100644
> --- a/hw/dmx/dmxscrinit.h
> +++ b/hw/dmx/dmxscrinit.h
> @@ -40,6 +40,11 @@
>
> #include "scrnintstr.h"
>
> +#define UNDEFINED -1
> +
> +#define MAXDEPTH 32
> +#define MAXVISUALSPERDEPTH 256
> +
> extern Bool dmxScreenInit(ScreenPtr pScreen, int argc, char *argv[]);
>
> extern void dmxBEScreenInit(ScreenPtr pScreen);
> --
> 1.8.5.5
>
>
[Attachment #5 (text/html)]
<div dir="ltr"><div>Please ignore the first patch, got old comment lines left in by \
accident. Probably forgot to git add.<br><br></div>Alex<br></div><div \
class="gmail_extra"><br><div class="gmail_quote">On Sun, Nov 30, 2014 at 2:37 PM, \
Alex Orange <span dir="ltr"><<a href="mailto:crazycasta@gmail.com" \
target="_blank">crazycasta@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><span class="">Likely fixes: <a \
href="https://bugs.freedesktop.org/show_bug.cgi?id=24642" \
target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=24642</a><br> <br>
dmx uses fb to handle many action. As such is uses fbScreenInit. The<br>
trouble is that fbScreenInit uses mi to generate the visuals. mi has<br>
trouble with 32-bit (rgba) depths. It tries to treat these depths as<br>
10-bits per pixels. This results in colormap_sizes in the visuals being<br>
2048 instead of 256. Also, the comments in micmap.c about the macros,<br>
_CE is the relevant one, suggest that these numbers may be driver<br>
defined. To this end, this patch simply copies the visuals into the<br>
visuals and depths structs instead of trying to coax mi into creating<br>
the right visuals. The code was mostly just taken from xnest with<br>
changes since dmx uses fb. The result depends on miScreenInit simply<br>
stuffing visuals in the screen and not doing anything more with them. In<br>
the future perhaps fbScreenInit can take an optional set of visuals to<br>
use instead of calling miInitVisuals.<br>
<br>
Signed-off-by: Alex Orange <<a \
href="mailto:crazycasta@gmail.com">crazycasta@gmail.com</a>><br>
Tested-by: Alex Orange <<a \
href="mailto:crazycasta@gmail.com">crazycasta@gmail.com</a>><br>
---<br>
</span> hw/dmx/dmxscrinit.c | 159 \
++++++++++++++++++++++++++++++++++++++++------------<br> hw/dmx/dmxscrinit.h | 5 \
++<br> 2 files changed, 129 insertions(+), 35 deletions(-)<br>
<br>
diff --git a/hw/dmx/dmxscrinit.c b/hw/dmx/dmxscrinit.c<br>
index 963d3a9..3c68708 100644<br>
<div><div class="h5">--- a/hw/dmx/dmxscrinit.c<br>
+++ b/hw/dmx/dmxscrinit.c<br>
@@ -54,9 +54,12 @@<br>
<br>
#include "dmxpict.h"<br>
<br>
-#include "fb.h"<br>
+#include <X11/X.h><br>
+#include "mi.h"<br>
#include "mipointer.h"<br>
#include "micmap.h"<br>
+#include "resource.h"<br>
+#include "fb.h"<br>
<br>
extern Bool dmxCloseScreen(ScreenPtr pScreen);<br>
static Bool dmxSaveScreen(ScreenPtr pScreen, int what);<br>
@@ -172,12 +175,28 @@ dmxBEScreenInit(ScreenPtr pScreen)<br>
}<br>
}<br>
<br>
+static int<br>
+offset(unsigned long mask)<br>
+{<br>
+ int count;<br>
+<br>
+ for (count = 0; !(mask & 1) && count < 32; count++)<br>
+ mask >>= 1;<br>
+<br>
+ return count;<br>
+}<br>
+<br>
/** Initialize screen number \a pScreen->myNum. */<br>
Bool<br>
dmxScreenInit(ScreenPtr pScreen, int argc, char *argv[])<br>
{<br>
DMXScreenInfo *dmxScreen = &dmxScreens[pScreen->myNum];<br>
- int i, j;<br>
+ int i, j, depthIndex;<br>
+ VisualPtr visuals;<br>
+ DepthPtr depths;<br>
+ int numVisuals, numDepths;<br>
+ VisualID defaultBEVisual, defaultVisual;<br>
+ int rootDepth;<br>
<br>
if (!dixRegisterPrivateKey(&dmxScreenPrivateKeyRec, PRIVATE_SCREEN, \
0))<br> return FALSE;<br>
</div></div>@@ -202,41 +221,95 @@ dmxScreenInit(ScreenPtr pScreen, int argc, char \
*argv[])<br> <div><div class="h5"> if (!dmxInitPixmap(pScreen))<br>
return FALSE;<br>
<br>
- /*<br>
- * Initalise the visual types. miSetVisualTypesAndMasks() requires<br>
- * that all of the types for each depth be collected together. It's<br>
- * intended for slightly different usage to what we would like here.<br>
- * Maybe a miAddVisualTypeAndMask() function will be added to make<br>
- * things easier here.<br>
- */<br>
- for (i = 0; i < dmxScreen->beNumDepths; i++) {<br>
- int depth;<br>
- int visuals = 0;<br>
- int bitsPerRgb = 0;<br>
- int preferredClass = -1;<br>
- Pixel redMask = 0;<br>
- Pixel greenMask = 0;<br>
- Pixel blueMask = 0;<br>
-<br>
- depth = dmxScreen->beDepths[i];<br>
- for (j = 0; j < dmxScreen->beNumVisuals; j++) {<br>
- XVisualInfo *vi;<br>
-<br>
- vi = &dmxScreen->beVisuals[j];<br>
- if (vi->depth == depth) {<br>
- /* Assume the masks are all the same. */<br>
- visuals |= (1 << vi->class);<br>
- bitsPerRgb = vi->bits_per_rgb;<br>
- redMask = vi->red_mask;<br>
- greenMask = vi->green_mask;<br>
- blueMask = vi->blue_mask;<br>
- if (j == dmxScreen->beDefVisualIndex) {<br>
- preferredClass = vi->class;<br>
- }<br>
+ visuals = (VisualPtr) malloc(dmxScreen->beNumVisuals * \
sizeof(VisualRec));<br> + numVisuals = 0;<br>
+<br>
+ depths = (DepthPtr) malloc(MAXDEPTH * sizeof(DepthRec));<br>
+ depths[0].depth = 1;<br>
+ depths[0].numVids = 0;<br>
+ depths[0].vids = (VisualID *) malloc(MAXVISUALSPERDEPTH * \
sizeof(VisualID));<br> + numDepths = 1;<br>
+<br>
+ defaultBEVisual = \
XVisualIDFromVisual(DefaultVisual(dmxScreen->beDisplay,<br> + \
DefaultScreen<br> + \
(dmxScreen->beDisplay)));<br> + rootDepth = UNDEFINED;<br>
+ defaultVisual = UNDEFINED;<br>
+<br>
+ for (i = 0; i < dmxScreen->beNumVisuals; i++) {<br>
+ visuals[numVisuals].class = dmxScreen->beVisuals[i].class;<br>
+ visuals[numVisuals].bitsPerRGBValue =<br>
+ dmxScreen->beVisuals[i].bits_per_rgb;<br>
+ visuals[numVisuals].ColormapEntries =<br>
+ dmxScreen->beVisuals[i].colormap_size;<br>
+ visuals[numVisuals].nplanes = dmxScreen->beVisuals[i].depth;<br>
+ visuals[numVisuals].redMask = dmxScreen->beVisuals[i].red_mask;<br>
+ visuals[numVisuals].greenMask = \
dmxScreen->beVisuals[i].green_mask;<br> + visuals[numVisuals].blueMask \
= dmxScreen->beVisuals[i].blue_mask;<br> + \
visuals[numVisuals].offsetRed =<br> + \
offset(dmxScreen->beVisuals[i].red_mask);<br> + \
visuals[numVisuals].offsetGreen =<br> + \
offset(dmxScreen->beVisuals[i].green_mask);<br> + \
visuals[numVisuals].offsetBlue =<br> + \
offset(dmxScreen->beVisuals[i].blue_mask);<br> +<br>
+ /* Check for and remove duplicates. */<br>
+ for (j = 0; j < numVisuals; j++) {<br>
+ if (visuals[numVisuals].class == visuals[j].class &&<br>
+ visuals[numVisuals].bitsPerRGBValue ==<br>
+ visuals[j].bitsPerRGBValue &&<br>
+ visuals[numVisuals].ColormapEntries ==<br>
+ visuals[j].ColormapEntries &&<br>
+ visuals[numVisuals].nplanes == visuals[j].nplanes \
&&<br> + visuals[numVisuals].redMask == \
visuals[j].redMask &&<br> + \
visuals[numVisuals].greenMask == visuals[j].greenMask &&<br> + \
visuals[numVisuals].blueMask == visuals[j].blueMask &&<br> + \
visuals[numVisuals].offsetRed == visuals[j].offsetRed &&<br> + \
visuals[numVisuals].offsetGreen == visuals[j].offsetGreen &&<br> + \
visuals[numVisuals].offsetBlue == visuals[j].offsetBlue)<br> + \
break;<br> + }<br>
+ if (j < numVisuals)<br>
+ break;<br>
+<br>
+ visuals[numVisuals].vid = FakeClientID(0);<br>
+<br>
+ depthIndex = UNDEFINED;<br>
+ for (j = 0; j < numDepths; j++)<br>
+ if (depths[j].depth == dmxScreen->beVisuals[i].depth) {<br>
+ depthIndex = j;<br>
+ break;<br>
}<br>
+<br>
+ if (depthIndex == UNDEFINED) {<br>
+ depthIndex = numDepths;<br>
+ depths[depthIndex].depth = dmxScreen->beVisuals[i].depth;<br>
+ depths[depthIndex].numVids = 0;<br>
+ depths[depthIndex].vids =<br>
+ (VisualID *) malloc(MAXVISUALSPERDEPTH * \
sizeof(VisualID));<br> + numDepths++;<br>
}<br>
- miSetVisualTypesAndMasks(depth, visuals, bitsPerRgb, preferredClass,<br>
- redMask, greenMask, blueMask);<br>
+ if (depths[depthIndex].numVids >= MAXVISUALSPERDEPTH) {<br>
+ FatalError("Visual table overflow");<br>
+ }<br>
+ depths[depthIndex].vids[depths[depthIndex].numVids] =<br>
+ visuals[numVisuals].vid;<br>
+ depths[depthIndex].numVids++;<br>
+<br>
+ if (dmxScreen->beVisuals[i].visualid == defaultBEVisual) {<br>
+ rootDepth = visuals[numVisuals].nplanes;<br>
+ defaultVisual = visuals[numVisuals].vid;<br>
+ }<br>
+<br>
+ numVisuals++;<br>
+ }<br>
+ visuals = (VisualPtr) realloc(visuals, numVisuals * sizeof(VisualRec));<br>
+<br>
+ if (rootDepth == UNDEFINED) {<br>
+ rootDepth = visuals[0].nplanes;<br>
+ }<br>
+ if (defaultVisual == UNDEFINED) {<br>
+ defaultVisual = visuals[0].vid;<br>
}<br>
<br>
</div></div> fbScreenInit(pScreen,<br>
@@ -245,6 +318,22 @@ dmxScreenInit(ScreenPtr pScreen, int argc, char *argv[])<br>
<span class=""> dmxScreen->scrnHeight,<br>
dmxScreen->beXDPI,<br>
dmxScreen->beXDPI, dmxScreen->scrnWidth, \
dmxScreen->beBPP);<br> +<br>
</span>+ // fbScreenInit calls miInitVisuals which makes uncontrollable \
colormap<br> + // sizes. The following overrides these visuals (pending a better \
method).<br> <div class="HOEnZb"><div class="h5">+ for (i = 0; i < \
pScreen->numDepths; i++) {<br> + \
free(pScreen->allowedDepths[i].vids);<br> + }<br>
+ free(pScreen->allowedDepths);<br>
+ free(pScreen->visuals);<br>
+<br>
+ pScreen->visuals = visuals;<br>
+ pScreen->numVisuals = numVisuals;<br>
+ pScreen->rootVisual = defaultVisual;<br>
+ pScreen->allowedDepths = depths;<br>
+ pScreen->numDepths = numDepths;<br>
+ pScreen->rootDepth = rootDepth;<br>
+<br>
(void) dmxPictureInit(pScreen, 0, 0);<br>
<br>
/* Not yet... */<br>
diff --git a/hw/dmx/dmxscrinit.h b/hw/dmx/dmxscrinit.h<br>
index 9fe9c98..23d2f62 100644<br>
--- a/hw/dmx/dmxscrinit.h<br>
+++ b/hw/dmx/dmxscrinit.h<br>
@@ -40,6 +40,11 @@<br>
<br>
#include "scrnintstr.h"<br>
<br>
+#define UNDEFINED -1<br>
+<br>
+#define MAXDEPTH 32<br>
+#define MAXVISUALSPERDEPTH 256<br>
+<br>
extern Bool dmxScreenInit(ScreenPtr pScreen, int argc, char *argv[]);<br>
<br>
extern void dmxBEScreenInit(ScreenPtr pScreen);<br>
--<br>
1.8.5.5<br>
<br>
</div></div></blockquote></div><br></div>
[Attachment #6 (text/plain)]
_______________________________________________
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic