Add 24-bit color support to exaGetPixmapFirstPixel

Submitted by Kevin Brace on Dec. 14, 2018, 4:32 a.m.

Details

Message ID 1544761947-1842-1-git-send-email-kevinbrace@gmx.com
State New
Headers show
Series "Add 24-bit color support to exaGetPixmapFirstPixel" ( rev: 1 ) in X.org

Not browsing as part of any series.

Commit Message

Kevin Brace Dec. 14, 2018, 4:32 a.m.
It appears that people who developed EXA forgot that there used to be
graphics devices that used 24-bits (3 bytes) instead of 32-bits (4 bytes)
in order to display one pixel. The lack of 24-bit color support inside
exaGetPixmapFirstPixel causes SiS 6326 to crash when running Xfce since
SiS 6326 does not support 32-bit color.

Signed-off-by: Kevin Brace <kevinbrace@gmx.com>
---
 exa/exa_unaccel.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/exa/exa_unaccel.c b/exa/exa_unaccel.c
index 73eada9..2d3e2ff 100644
--- a/exa/exa_unaccel.c
+++ b/exa/exa_unaccel.c
@@ -703,6 +703,7 @@  exaGetPixmapFirstPixel(PixmapPtr pPixmap)
 {
     switch (pPixmap->drawable.bitsPerPixel) {
     case 32:
+    case 24:
     {
         CARD32 pixel;
 

Comments

On Thu, 2018-12-13 at 22:32 -0600, Kevin Brace wrote:
> It appears that people who developed EXA forgot that there used to be
> graphics devices that used 24-bits (3 bytes) instead of 32-bits (4 bytes)
> in order to display one pixel. The lack of 24-bit color support inside
> exaGetPixmapFirstPixel causes SiS 6326 to crash when running Xfce since
> SiS 6326 does not support 32-bit color.

fb doesn't support 24bpp anymore, so this patch isn't going to do
anything useful I don't think.

- ajax
Hi Adam,

Not that it matters, but I am actually writing this e-mail from a computer that has SiS6326 in it.
It is an Intel Celeron 2.0A GHz (Pentium 4 based) computer along with a mainboard support for "legacy" (i.e., 3.3 V signaling) AGP cards.
The mainboard even has support for ACPI S3 State (Suspend to RAM), and while the SiS DDX UMS code for SiS6326 is not perfect, I can get the desktop back after standby resume if I cycle through VT and back to desktop.
Actually, many, many PCI and AGP cards of that era will go nuts when the computer comes out of standby (i.e., RAGE 128), so this is pretty good.
The particular card I have uses really cheap looking 2 layer PCB . . . (i.e., no power and ground inner conducting layer to reduce noise like 4 layer PCB has), but the graphics card appears to work fine somehow. (the picture is okay)
No crashes for the past 4 days I have been doing DDX related cleanup work. (i.e., SiS, Trident, and Alliance Semiconductor DDX patches I just posted)
Not bad as far as I am concerned considering how lousy looking the card's construction is.
    Anyway, the only way I can use such an old AGP graphics card with Xubuntu 16.04.5 (X Server 1.19 and Xfce) is because I limit the color bits per pixel (bpp) to 16 bit.
I have to use xorg.conf for this where I rather not.
    If I were to explain the situation in more details, if I do not use xorg.conf, it can display Xubuntu login screen, but as soon as it gets to the desktop, the X Server will crash.
It is precisely crashing at exaGetPixmapFirstPixel.
I will assume that when Xfce tries to display the desktop icons, it crashes the X Server due to the lack of 24 bpp support inside exaGetPixmapFirstPixel.
The rendering is fine for the login screen as far as I am concerned, so I would imagine that EXA can theoretically support 24 bpp, but no one really tried since by the time EXA was developed, every new graphics had 32 bpp support, and developers "forgot" to support 24 bpp only graphics cards.
I do not see any particular harm backporting that one line inside the still supported X Servers. (1.16 through 1.20)

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Monday, December 17, 2018 at 1:52 PM
> From: "Adam Jackson" <ajax@nwnk.net>
> To: "Kevin Brace" <kevinbrace@gmx.com>, xorg-devel@lists.x.org
> Subject: Re: [PATCH] Add 24-bit color support to exaGetPixmapFirstPixel
>
> On Thu, 2018-12-13 at 22:32 -0600, Kevin Brace wrote:
> > It appears that people who developed EXA forgot that there used to be
> > graphics devices that used 24-bits (3 bytes) instead of 32-bits (4 bytes)
> > in order to display one pixel. The lack of 24-bit color support inside
> > exaGetPixmapFirstPixel causes SiS 6326 to crash when running Xfce since
> > SiS 6326 does not support 32-bit color.
> 
> fb doesn't support 24bpp anymore, so this patch isn't going to do
> anything useful I don't think.
> 
> - ajax
> 
>
On Mon, 2018-12-17 at 23:54 +0100, Kevin Brace wrote:

> The rendering is fine for the login screen as far as I am concerned,
> so I would imagine that EXA can theoretically support 24 bpp, but no
> one really tried since by the time EXA was developed, every new
> graphics had 32 bpp support, and developers "forgot" to support 24
> bpp only graphics cards.

exa definitely had intended to support 24bpp at one point. I deleted
the code for it in 1.20 though:

https://gitlab.freedesktop.org/xorg/xserver/commit/0803918e64262482035f042e5e1f2a571d3dea1b

> I do not see any particular harm backporting that one line inside the
> still supported X Servers. (1.16 through 1.20)

The change would do nothing for 1.20, and I'm not about to do any more
pre-1.20 releases. If someone else wants to, be my guest.

- ajax