[rdesktop-devel] [ rdesktop-Patches-1390148 ] Refactoring of color depth code

[SourceForge.net <no...@so...>]

Patches item #1390148, was opened at 2005-12-25 19:18
Message generated for change (Comment added) made by astrand
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=381349&aid=1390148&group_id=24366

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Ilya Konstantinov (ikonst)
Assigned to: Peter Åstrand (astrand)
Summary: Refactoring of color depth code

Initial Comment:
This patch:

1. Refactors the code which chooses the best visual.
The code was changed to ensure a visual which allows
bitwise copying of bitmaps from Windows to X pixmaps to
be used in preference to all others. Otherwise, it
makes sure to chose the best visual, as long as it's
not a problematic >24bpp visual. If a TrueColor visual
is not available, it picks an 8-plane
256-colormap-entries PseudoColor visual, whcih is the
only pseudocolor (colormap'd) visual we support. All in
all, we support 8, 15, 16 and 24 color depths. In all
other cases, we'll avoid running at all, presenting an
intelligible error message (instead of being unable to
draw and just displaying a blank window).

2. Rephrases error messages to be more precise.

3. Makes a clear distinction between the terms "depth"
and "bpp": depth is the meaningful bits, bpp is the
actual bits per pixel, including alignment. The patch
makes sure the correct terms are used throughout
variable names and messages.

4. Eliminates some compilation warnings along the way :)

The patch was tested on:
- 24-plane R8G8B8 visual with 32bpp pixmaps
- 16-plane R5G6B5 visual with 16bpp pixmaps
- 15-plane R5G5B5 visual with 16bpp pixmaps
- 8-plane 256-colormap-size visual

Whenever available, I made sure to test the optimized
(no-bitmap-translation) code path.

----------------------------------------------------------------------

>Comment By: Peter Åstrand (astrand)
Date: 2006-01-05 13:38

Message:
Logged In: YES 
user_id=344921

The attached patch is slightly modified:

* All the visual select stuff has been lifted out to a
separate function. ui_init became very long...

* Indent fixes, by running indent all. 

I'm also wondering: rdesktop is currently using depth 8 as
default. Does this really make sense? I suggest that we
change this to 16, 24 or 32, if the fallback mechanism works
correctly. 

----------------------------------------------------------------------

Comment By: Peter Åstrand (astrand)
Date: 2006-01-05 13:34

Message:
Logged In: YES 
user_id=344921

With this patch, the graphic is distorted when running on a
16-bit display against a server with 8bpp. Also, valgrind
reports errors in this case. The attached screenshot shows
the problem. 

----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2006-01-03 17:03

Message:
Logged In: YES 
user_id=335423

Peter, to quote you:
"If you are able to come up with a patch that solves real
problems and that makes, I think we should apply it."
:)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=381349&aid=1390148&group_id=24366