search

ghaerr / microwindows

The Nano-X Window System
Other
648 stars 91 forks source link

Addition palette features #42

Closed djipi closed 3 years ago

djipi commented 3 years ago

Added CreatePalette, SelectPalette, RealizePalette, SetSystemPaletteUse, and SetDIBColorTable functions. Added the NOSTDPALX compilation directives.

ghaerr commented 3 years ago

Hello @djipi,

Thanks for the Win32 palette functions!

Things look great, with the exception of the following:

1) All of the #ifdef NOSTDPALX in genmem.c: the GdCalcMemGCAlloc function is used on all systems to compute the pitch (and resulting image size) for any image encountered, regardless of the possible linked-in screen capabilities. Thus, I don't believe these should be #ifdef'd.

2) Lines 198-217 of fb.c: same reason as above. The set_data_formatex function is used for images opened regardless of current linked-in screen settings, so should not be #ifdef'd. The lines 116-130 are OK, as those will remove unused sub drivers that may never get used, although some caution is advised since internal blitters could still use these sub drivers for offscreen memory blitting and image conversion. Lines 131-135 should probably not be #ifdef'd, since the TRUECOLOR (non-palettized) fblin8 subdriver is used for them even though the device screen may be palettized.

In general, I'm not sure how good an idea it is to #ifdef out items based on NOSTDPALX, unless code space is truly at a premium, as internal image, blit and conversion functionality could be lost.

Do you plan on finishing the functions where a #pragma message is emitted during each compilation? If so, that's fine leaving them in, otherwise they should probably be removed.

djipi commented 3 years ago

Hi @ghaerr

I have removed the couple of pragma message and the NOSTDPALX compilation directives.

ghaerr commented 3 years ago

Hello @djipi,

This looks great, thank you. I am ready to commit it.

In deference to @rofl0r's concerns and work trying to keep image sizes small in the repo, I have not yet heard back from him on whether he has time or interest in adjusting the repo using some git magic. I would like to hold this PR until he responds, if that's ok with you. Go ahead and start preparations for your next PR, thank you!

djipi commented 3 years ago

Sure @ghaerr No problem, I will do the next PR from another branch.