Big DIB fix

François Gouget fgouget at codeweavers.com
Wed Oct 17 01:47:14 CDT 2001


Changelog:

   François Gouget <fgouget at codeweavers.com>

 * graphics/x11drv/dib.c

   Fix tons of color conversion bugs
   Reorganize things more rationally and so that more code is shared



   This is a big (5700+ lines) patch. Why is it so big? Because it fixes
lots of errors (I have lost count) and reorganizes things more
rationally so that more code is shared.
   Here is an incomplete list of the kinds of things that it fixes:
 * SetImageBits_16, X in 15bpp. If the dib is not exactly in the same
format as X, then we assume it is an rgb565 dib. But it could also be a
bgr565, rgb555 or bgr555 DIB!
 * SetImageBits_16, X in 16bpp. Same thing, etc... etc. Let's skip to
another kind of bug.
 * SetImageBits_16. Whether X is in 24bpp or 32bpp we execute the same
code. This assumes that whenever X is in 24bpp mode it is padded to 32
bits. This is not necessarily the case! If bmpImage->depth==24 and
bmpImage->bits_per_pixel==24, then it is a 'packed' 24bpp format which
requires separate code. Otherwise we must fallback on to the 32bpp case.
This bug is pervasive.
 * Still in this case 32:, the first branch of the if handles two types
of 16 bit DIBs (out of 4), the second one handles just one (still out of
four).
 * SetImageBits_32 assumes that there is only one supported format of 32
bit DIB. In fact windows applications are free to set their masks about
any way they want. For instance I have seen r=0xff000000, g=0x00ff0000
and b=0x0000ff00 ! Of course for efficiency it is a good idea to special
case the more common rgb0888 and bgr0888 cases.
 * There are a couple of bugs here and there where colors are shifted by
one bit too many or anded with a slightly wrong mask. I won't give
examples: these are hard to spot.
 * Comments are inconsistent, especially wrt. rgb vs. bgr: one says
red_mask == 0xf800 means 565bgr (l2111) while another says red_mask ==
0x001f means 565bgr (l1898). RGB is when the red color is in the high
order bits of the mask. This seems to be confirmed by both windows and
vncserver.

   It's a large patch to I tested it a lot. I wrote a test application
that converts a crafted test image between the following DIB formats:
rgb555, bgr555, rgb565, bgr565, rgb888, rgb0888, bgr0888, rgb8880,
bgr8880.
   But to really test DIBs you must also vary the X screen depth. So I
ran it in the following X settings (the X driver/server is indicated in
parenthesis):
   pal    1     (vga)
   pal    4     (vga)
   pal    8     (vesa)
   rgb  233     (vnc)
   rgb  555     (neomagic, vnc)
   bgr  555     (vnc)
   rgb  565     (neomagic, vesa, vnc)
   bgr  565     (vnc)
   rgb  888     (vesa)
   rgb  888/32  (neomagic, vnc)
   bgr  888/32  (vnc)
   rgb 0888     (vnc)
   bgr 0888     (vnc)

   I compared the log produced by the test application against the logs
produced on nt4 and they match... modulo the color precision loss that
we incur when X is in a low color resolution mode. We should really have
a DIB engine...

   In the new code I moved most of the conversion code to
X11DRV_DIB_Convert_xxx function that can then be shared between
different conversion cases to/from DIBs and XImages. There is probably
more work that can be done in the code that calls out to them to
factorize a bit more code. It is probably even be possible, to set up
some sort of array of pointer to function indexed by the type of source
and destination. For this splitting desc into two instances of a
structure that can describe a DIB as well as an XImage would be great.
But that's a patch for another day.


-- 
François Gouget
fgouget at codeweavers.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p20011016-dib.diff.gz
Type: application/x-gzip
Size: 20673 bytes
Desc: not available
Url : http://www.winehq.org/pipermail/wine-patches/attachments/20011016/2f87ad50/p20011016-dib.diff.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dib.c
Type: text/x-csrc
Size: 12390 bytes
Desc: not available
Url : http://www.winehq.org/pipermail/wine-patches/attachments/20011016/2f87ad50/dib.c


More information about the wine-patches mailing list