Added VGA/VESA modes. Fix Attribute Configuration FIXMEs
Rob Shearman
robertshearman at gmail.com
Tue Oct 7 11:35:43 CDT 2008
Hi Jeremiah,
Thank you for your contribution to Wine. However, I have some comments
about your patch:
2008/10/7 Jeremiah Flerchinger <jeremiah.flerchinger at gmail.com>:
> Added additional VGA/VESA modes. Added missing information to VGA/VESA mode descriptions.
> Used new mode information to fix attributes incorrectly set in int10.c
> Moved VGA/VESA mode descriptions to vga.h, so they can be shared by emulated VGA/VESA hardware and
> BIOS graphics routines.
> ---
> dlls/winedos/int10.c | 149 +++++++++++++++++++------------------------------
> dlls/winedos/vga.h | 90 ++++++++++++++++++++++++++++++
> 2 files changed, 148 insertions(+), 91 deletions(-)
> mode change 100644 => 100755 dlls/winedos/int10.c
Try not to change file modes unnecessarilly.
> diff --git a/dlls/winedos/int10.c b/dlls/winedos/int10.c
> old mode 100644
> new mode 100755
> index 94ea6ff..554b083
> --- a/dlls/winedos/int10.c
> +++ b/dlls/winedos/int10.c
> @@ -4,6 +4,8 @@
> * Copyright 1998 Ove Kven
> * Copyright 1998 Joseph Pranevich
> *
> + * 10/2008 - Jeremiah Flerchinger - Extended VGA/VESA mode configuration support
> + *
The GIT changelog will tell everyone what code you have added so there
is no need to state it at the top. If you feel that a significant
portion of the file is your copyright now then feel free to add a
copyright line similar to the others above.
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> * License as published by the Free Software Foundation; either
> @@ -218,7 +155,9 @@ static const INT10_MODE *INT10_FindMode( WORD mode )
> while (ptr->Mode != 0xffff)
> {
> if (ptr->Mode == mode)
> + {
> return ptr;
> + }
> ptr++;
> }
>
This change is unnecessary.
> @@ -343,22 +282,34 @@ static BOOL INT10_FillModeInformation( struct _ModeInfoBlock *mib, WORD mode )
> * 13-15 - Reserved.
> */
> {
> - WORD attr = 0x000a; /* color mode, optional info */
> + WORD attr = 0x0002; /* set optional info available */
> + /* Enable color mode - if both Colors & Depth are 0, then assume monochrome*/
> + if (ptr->Colors || ptr->Depth)
> + {
> + attr |= 0x0008;
> + }
>
> /*
> * FIXME: Attribute handling is incomplete.
> */
>
> - /* Mode supported? FIXME: correct value */
> - attr |= 0x0001;
> + /* Mode supported? */
> + if (ptr->Supported)
> + {
> + attr |= 0x0001; // set display mode supported
C99-style comments are not portable as they don't work on some older
compilers. Use "/* */" instead.
..
> diff --git a/dlls/winedos/vga.h b/dlls/winedos/vga.h
> index 36646a1..317b9ca 100644
> --- a/dlls/winedos/vga.h
> +++ b/dlls/winedos/vga.h
> @@ -3,6 +3,7 @@
> *
> * Copyright 1998 Ove Kven
> *
> + *
Please fix this spurious whitespace change.
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> * License as published by the Free Software Foundation; either
...
> +static const VESA_MODE VESA_modelist[] =
Don't put data in header files. If it needs to be shared, make it
non-static in the original file and add an extern declaration for it.
> +{
> + /* VGA modes */
> + {0x0000, TEXT, 40, 25, 9, 16, 360, 400, 0, 16, 8, TRUE}, /* VGA text mode 0 */
> + {0x0001, TEXT, 40, 25, 9, 16, 360, 400, 0, 16, 8, TRUE}, /* VGA text mode 1 */
> + {0x0002, TEXT, 80, 25, 9, 16, 360, 400, 0, 16, 8, TRUE}, /* VGA text mode 2 */
> + {0x0003, TEXT, 80, 25, 9, 16, 360, 400, 0, 16, 8, TRUE}, /* VGA text mode 3 */
> + {0x0004, GRAPHIC, 40, 25, 8, 8, 320, 200, 2, 4, 1, FALSE}, /* VGA graphics mode 4 */
> + {0x0005, GRAPHIC, 40, 25, 8, 8, 320, 200, 2, 4, 1, FALSE}, /* VGA graphics mode 5 */
> + {0x0006, GRAPHIC, 80, 25, 8, 8, 640, 200, 1, 2, 1, FALSE}, /* VGA graphics mode 6 */
> + {0x0007, TEXT, 80, 25, 9, 16, 720, 400, 0, 0, 8, FALSE}, /* VGA text mode 7 - FIXME bad default address */
> + {0x000d, GRAPHIC, 40, 25, 8, 8, 320, 200, 4, 16, 8, FALSE}, /* VGA graphics mode 13 */
> + {0x000e, GRAPHIC, 80, 25, 8, 8, 640, 200, 4, 16, 4, FALSE}, /* VGA graphics mode 14 */
> + {0x000f, GRAPHIC, 80, 25, 8, 14, 640, 350, 0, 0, 2, FALSE}, /* VGA graphics mode 15 */
> + {0x0010, GRAPHIC, 80, 25, 8, 14, 640, 350, 4, 16, 2, FALSE}, /* VGA graphics mode 16 */
> + {0x0012, GRAPHIC, 80, 30, 8, 16, 640, 480, 1, 2, 1, FALSE}, /* VGA graphics mode 17 */
> + {0x0012, GRAPHIC, 80, 30, 8, 16, 640, 480, 4, 16, 1, FALSE}, /* VGA graphics mode 18 */
> + {0x0013, GRAPHIC, 40, 25, 8, 8, 320, 200, 8, 256, 1, TRUE}, /* VGA graphics mode 19 */
> + /* VESA 7-bit modes */
> + {0x006a, GRAPHIC, 0, 0, 0, 0, 800, 600, 4, 16, 1, TRUE}, /* VESA graphics mode, same as 0x102 */
> + /* VESA 15-bit modes */
> + {0x0100, GRAPHIC, 0, 0, 0, 0, 640, 400, 8, 256, 1, TRUE}, /* VESA graphics mode */
> + {0x0101, GRAPHIC, 0, 0, 0, 0, 640, 480, 8, 256, 1, TRUE}, /* VESA graphics mode */
> + {0x0102, GRAPHIC, 0, 0, 0, 0, 800, 600, 4, 16, 1, TRUE}, /* VESA graphics mode */
> + {0x0103, GRAPHIC, 0, 0, 0, 0, 800, 600, 8, 256, 1, TRUE}, /* VESA graphics mode */
> + {0x0104, GRAPHIC, 0, 0, 0, 0, 1024, 768, 4, 16, 1, TRUE}, /* VESA graphics mode */
> + {0x0105, GRAPHIC, 0, 0, 0, 0, 1024, 768, 8, 256, 1, TRUE}, /* VESA graphics mode */
> + {0x0106, GRAPHIC, 0, 0, 0, 0, 1280, 1024, 4, 16, 1, TRUE}, /* VESA graphics mode */
> + {0x0107, GRAPHIC, 0, 0, 0, 0, 1280, 1024, 8, 256, 1, TRUE}, /* VESA graphics mode */
> + {0x0108, TEXT, 0, 0, 0, 0, 80, 60, 0, 0, 1, TRUE}, /* VESA text mode */
> + {0x0109, TEXT, 0, 0, 0, 0, 132, 25, 0, 0, 1, TRUE}, /* VESA text mode */
> + {0x010a, TEXT, 0, 0, 0, 0, 132, 43, 0, 0, 1, TRUE}, /* VESA text mode */
> + {0x010b, TEXT, 0, 0, 0, 0, 132, 50, 0, 0, 1, TRUE}, /* VESA text mode */
> + {0x010c, TEXT, 0, 0, 0, 0, 132, 60, 0, 0, 1, TRUE}, /* VESA text mode */
> + /* VESA 1.2 modes */
> + {0x010d, GRAPHIC, 0, 0, 0, 0, 320, 200, 15, 0, 1, TRUE}, /* VESA graphics mode, 32K colors */
> + {0x010e, GRAPHIC, 0, 0, 0, 0, 320, 200, 16, 0, 1, TRUE}, /* VESA graphics mode, 64K colors */
> + {0x010f, GRAPHIC, 0, 0, 0, 0, 320, 200, 24, 0, 1, TRUE}, /* VESA graphics mode, 16.8 Million colors */
> + {0x0110, GRAPHIC, 0, 0, 0, 0, 640, 480, 15, 0, 1, TRUE}, /* VESA graphics mode, 32K colors */
> + {0x0111, GRAPHIC, 0, 0, 0, 0, 640, 480, 16, 0, 1, TRUE}, /* VESA graphics mode, 64K colors */
> + {0x0112, GRAPHIC, 0, 0, 0, 0, 640, 480, 24, 0, 1, TRUE}, /* VESA graphics mode, 16.8 Million colors */
> + {0x0113, GRAPHIC, 0, 0, 0, 0, 800, 600, 15, 0, 1, TRUE}, /* VESA graphics mode, 32K colors */
> + {0x0114, GRAPHIC, 0, 0, 0, 0, 800, 600, 16, 0, 1, TRUE}, /* VESA graphics mode, 64K colors */
> + {0x0115, GRAPHIC, 0, 0, 0, 0, 800, 600, 24, 0, 1, TRUE}, /* VESA graphics mode, 16.8 Million colors */
> + {0x0116, GRAPHIC, 0, 0, 0, 0, 1024, 768, 15, 0, 1, TRUE}, /* VESA graphics mode, 32K colors */
> + {0x0117, GRAPHIC, 0, 0, 0, 0, 1024, 768, 16, 0, 1, TRUE}, /* VESA graphics mode, 64K colors */
> + {0x0118, GRAPHIC, 0, 0, 0, 0, 1024, 768, 24, 0, 1, TRUE}, /* VESA graphics mode, 16.8 Million colors */
> + {0x0119, GRAPHIC, 0, 0, 0, 0, 1280, 1024, 15, 0, 1, TRUE}, /* VESA graphics mode, 32K colors */
> + {0x011a, GRAPHIC, 0, 0, 0, 0, 1280, 1024, 16, 0, 1, TRUE}, /* VESA graphics mode, 64K colors */
> + {0x011b, GRAPHIC, 0, 0, 0, 0, 1280, 1024, 24, 0, 1, TRUE}, /* VESA graphics mode, 16.8 Million colors */
> + {0xffff, TEXT, 0, 0, 0, 0, 0, 0, 0, 0, 1, FALSE}
> +};
> +
> +/* True if video mode is a vesa mode, false otherwise.
> + * More correct would be to use something like (x > 0xff || x == 0x6a)
> + * but as long as we have only the standard VGA and VESA modes this is ok too */
> +#define IS_VESA_MODE(x) ((x) >= 0x6a)
> +
In general, you want to submit many small patches instead of one
larger patch. Some of the things you are doing are renames of
functions or variables and so could be done independently of
functional changes. This will make the patch easier to review and
hence more likely to get in quickly.
--
Rob Shearman
More information about the wine-devel
mailing list