[haiku-development] Re: [PATCH] Add 8-bit indexed colour support to boot splash
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Tue, 29 Jul 2008 10:52:27 +0200 CEST
Hi David,
"David Powell" <david@xxxxxxxxxxxxxxxxx> wrote:
> This proposed patch adds 8-bit indexed colour support to the
> bootloader
> and kernel, so that the safemode VESA 8-bit colour modes can display
> the boot splash screen.
Sorry for the delay, I didn't find much time in the last couple of
days, and I'm still catching up mails.
> Also included are additions to the generate_boot_screen program to
> add the indexed images and colour palette information to the images.h
> file.
Nice! There are a couple of coding style issues, like:
> + if (buffer[0] > 1) {
> + fprintf (sOutput, "%d, ",
> + 128 + buffer[0] - 1);
> + new_line_if_required();
> + for (int i = 1; i < buffer[0] ;
> i++) {
> + fprintf( sOutput, "%d,
> ",
> + buffer[i] );
Spacing of the function calls is wrong - it's always fprintf("") not
fprintf ("") nor fprintf( "" ) :-)
Also, looking at the end of the patch, the output looks wrong, too, as
there is an additional tab before the last '};'.
Other than that, the patch looks good to me.
> I have also added the new output of the generate_boot_screen to the
> patch
> as a new images.h file.
>
> While the bootloader and kernel patches are feature complete, the
> generate_boot_screen program currently does not output a nice colour
> mapped indexed image, and instead outputs a greyscale image of the
> logo and icons. It's enough to check the bootloader and kernel code
> work
> correctly, but doing a 24-bit to 8-bit image conversion was way more
> work that I expected, so I added in a greyscale conversion as a stop-
> gap
> measure.
Grayscale is probably enough for the moment. You could do a simple
palette replacement algorithm instead of a real color conversion,
though -- dunno if that would look better, though.
If we just do a simple conversion like this, I would actually prefer to
do it "live" in the boot loader, though; at least it should consume
less space, and should still be pretty much instant.
> The Run Length Encoding routines in both the bootloader and
> generate_boot_screen are contained in seperate 24 and 8 bit
> functions,
> rather than one function with a parameter denoting depth, as I want
> to
> add VGA 640x480x4 video mode support and (possibly) 4 bit RLE before
> simplifying
> the code.
Sounds good!
Bye,
Axel.
Other related posts:
