AGS4: Extend Allegro palette to 0-255

[AlanDrake]

This is a WIP attempt to increase Allegro's internal palette from 0-63 to 0-255.

EDIT2: I managed to fix the bestfit algorithm, but I'm still converting to 6bit for the rbg_map

I upgraded some pieces from the ancient demo quest, which can be used as a test project. DemoQuest_ags4.zip

You can also import other 256 games, but they will need a full rebuild to update the palettes in game data, and manual palette manipulation that were previously restricted to 0-63 will obviously work differently.

[ericoporto]

I am curious about this being rebased after the recent changes.

[AlanDrake]

Alright, I rebased and revised this PR. I made create_color_table and bestfit_color create a temporary 6bit palette for compatiblity with their uber haxx algorithms, which I'm not good enough to upgrade.

Should work fine both for antialiased sprites on and off. I didn't test BMP/PCX/FLI load/save/display.

[ivan-mogilko]

I will try to finally review this in the following days.

I upgraded some pieces from the ancient demo quest, which can be used as a test project.

Idk if useful, but a while ago I added "test--4.0.0" branch to the old demo repository: https://github.com/adventuregamestudio/ags-demo-quest-old/tree/test--4.0.0

[ivan-mogilko]

Besides the in-code comments above,

1. Game data version (GameDataVersion enum) should be raised, because palette unit range has changed. Project version may be left, I think, as palette was saved in 256-unit range there.
2. For a proper behavior, there should be a palette upgrade from 64-range to 256-range on game load in the engine (UpdateGameData) if loaded a game of lower version.
3. "Game State" save component version should be raised, for the same reasons, as palette is written there. And palette upgraded on loading an older save.
4. I am bothered by temporary palette in bestfit_color, because that function is run per pixel, meaning this temporary palette will also be recreated over and over again for each processed pixel. We need to find way to optimize this, such as create a fixed palette beforehand and pass into this function instead of original palette.
5. I am also bothered by the potential use of rgb_map in fixup_loaded_bitmap, which I do not fully understand. But it looks like the engine only goes this path if a hi/true-color image is being converted to 8-bit image, which is unlikely to happen in practice.
6. There's also BestFitColor in PaletteUtilities.cs, which i ported to C# for remapping backgrounds. Likely it should have same fix?
7. ~Need to test FLIC playback, to see if its looks changed (but I guess the worst that may happen is that there will be more color variation?) EDIT: well, it looks fine at the first glance.~
8. There's still this statement in the PR's description:

   It should execute as expected, but will crash when enabling sprite antialiasing, which calls the unfinished create_rgb_table function.

Is this still true, or has been fixed?

BTW, I got confused by SetPalRGB script function. The manual states that "The RED, GREEN and BLUE parameters each range from 0 to 63". But the function itself has no parameter validation whatsoever. Git Blame shows no changes done to this function, nor to wsetrgb function in the last 12 years. Which means that this function was unsafe all this time, if user tries to pass any invalid value (negative, or >63) there, the engine will malfunction or crash.

[AlanDrake]

Good news.

Thanks to divine inspiration I managed to fix the bestfit algorithms for real. Not sure if formally correct, but I finally found the weights and values to make it looks like it's working without glaring color mismatches.

Now sprites can finally match the 252-gray that used to become full white.

The rgb_table is still being wrapped to a 6bit palette. I don't have time for figuring that one. I duped the bestfit_init, so it can still generate the weights it needs.

1. Game data version (GameDataVersion enum) should be raised, because palette unit range has changed. Project version may be left, I think, as palette was saved in 256-unit range there.

2. For a proper behavior, there should be a palette upgrade from 64-range to 256-range on game load in the engine (UpdateGameData) if loaded a game of lower version.

3. "Game State" save component version should be raised, for the same reasons, as palette is written there. And palette upgraded on loading an older save.

Seems a waste of time, since the format is essentially unchanged and we only need a full rebuild to get everything on track. Savestates should be expected to be unsafe on a alpha build.

4. I am bothered by temporary palette in bestfit_color, because that function is run per pixel, meaning this temporary palette will also be recreated over and over again for each processed pixel. We need to find way to optimize this, such as create a fixed palette beforehand and pass into this function instead of original palette.

Sounds good, but in my opinion supporting 8bit mode in the way it's implemented is a waste of time, because shaders could do all these transformations in more convenient ways.

I suppose that's what the rgb_table is supposed to optimize. In fact there are several sections where the table overtakes the individual bestfit, if initialized.

5. I am also bothered by the potential use of rgb_map in fixup_loaded_bitmap, which I do not fully understand. But it looks like the engine only goes this path if a hi/true-color image is being converted to 8-bit image, which is unlikely to happen in practice.

I'm bothered by all the 8bit code. I hope to never have to touch it again.

6. There's also BestFitColor in PaletteUtilities.cs, which i ported to C# for remapping backgrounds. Likely it should have same fix?

I managed to fix both to full 0-255 color matching.

8. There's still this statement in the PR's description:

Fixed

[ivan-mogilko]

Seems a waste of time, since the format is essentially unchanged and we only need a full rebuild to get everything on track.

Format is changed, because you switch the palette units from 64-based to 256-based, this changes the meaning of data. Without fixups this will make loading older data incorrect. You may ignore upgrading older data in the engine, but at least the version should be incremented to indicate the change in format. This will help in case someone will like to analyze game data created by different versions.

EDIT: Fine, I will raise it myself after merging this.

[ivan-mogilko]

Tested the palette remap, it seems to work fine (results are equal or close to the existing version).

[ivan-mogilko]

Somehow I completely missed this, but this PR did 2 changes that were not related to the palette in any way, and instead changed regular color property calculation from 16-bit R5G6B5 to 32-bit R8G8B8, which refers rather to task #1980. But since these changes are not complemented by others, on their own they break some things in both Editor and Engine.

Noteably these are changes to ColorMapper and __my_setcolor function in AGS.Native.

For example, same "Color" property numbers now produce a different RGB, so loading existing project suddenly makes colored things look different. And because __my_setcolor was only modifed in AGS.Native, but not Engine, they will also look different between runtime and the preview in the Editor.

I've already reverted the __my_setcolor back during small recent refactor, now this function is shared, but ColorMapper still has this change. I started working on #1980 now, where this change will make sense, but I am tempted to revert it temporarily, because #1980 may take time, and any public ags4 release done now will contain this discrepancy.

[ericoporto]

Uhm, is this also related to https://github.com/adventuregamestudio/ags/issues/2352 somehow?

[ivan-mogilko]

Uhm, is this also related to #2352 somehow?

This may be the related place in code, but the change itself is not related, because it was done in ags4, while the mistake is found in 3.* branch.