adventuregamestudio / ags

AGS editor and engine source code
Other
708 stars 159 forks source link

AGS 4: allow to import sprites as 8-bit image regardless of game color depth #2460

Closed ivan-mogilko closed 4 months ago

ivan-mogilko commented 5 months ago

This complements #2455.

Adds a combobox to the "Import Sprite" window that lets select whether to import the sprite, converting to game's default color depth (default choice), or import as a 8-bit image. This list may potentially be expanded, if necessary. spriteimport--importdepth2

The choices are currently named:

At first I wanted the second option to be displayed only if the imported image is of indexed pixel format, but this may be complicated to do, as multiple different files may be selected at once.

Removed "Import alpha channel" option, as redundant.

Other things... Sprite index file now stores additional array of bytes, defining sprite's color depth. This is to let know the sprite's color depth without loading it up.


NOTE: for testing 8-bit sprites as walkable masks following settings must be used:

ivan-mogilko commented 5 months ago

Also, maybe it's the time to remove "Import alpha channel" option? How useful it is anyway? The only difference it currently makes is that if sprite has <255 alpha anywhere, this option will turn the image to full opaque. But I suppose that if user wants an opaque image, then they should make one in the first place.

ericoporto commented 4 months ago

ahn, how is this supposed to work? I tried to add an 8-bit color sprite (test8bit.zip this png) and the colors and got the following

Color 0,0=16
Color 146,64=18
Color 288,182=10

by using

  dynspr = DynamicSprite.CreateFromExistingSprite(1, eColorFmt_8bit);
  DrawingSurface* surf = dynspr.GetDrawingSurface();
  int c1 = surf.GetPixel(0, 0);
  int c2 = surf.GetPixel(146, 64);
  int c3 = surf.GetPixel(288, 182);
  System.Log(eLogInfo, "Color 0,0=%d\nColor 146,64=%d\nColor 288,182=%d", c1, c2, c3);

I expected to get 0, 1, and 9 which are the indexes of the colors. :/

Like the 8-bit sprite apparently shows with the same colors as I saved but the color that I get from GetPixel aren't the same as the index, so I am not sure what those numbers means. Edit: I tried disabling sprite storage optimization but it isn't that what causes whatever is happening. Using the mask pathfinder in the sprite also allows the character to walk anywhere, hinting that color at index 0 isn't 0 either.

Edit2: I think this is one of the things of the weird way the allegro palette handling works.

Edit3: erh, adding more colors, it looks like there is something more going on... I don't care for the colors not matching the original if the palette is being thrown away, but it's weird they aren't matching in index with the game palette, it looks like it's rematching the game palette but not by index and instead some sort of distance approximation.

image

NOTE: for testing 8-bit sprites as walkable masks following settings must be used

this doesn't look sufficient to achieve this, tried also with different settings and couldn't figure it out.

ivan-mogilko commented 4 months ago

We need to test this step by step:

  1. Original sprite. It's better to use something suitable for precise testing. Here's the one I use: a256small.zip it's a tiny 10x1 bitmap where each pixel has a different color ranging from 0 to 8.

  2. Importing sprite. When importing I double check that the source image is detected as "8-bit indexed", which is correct. Then I use following import settings:

    • Import color depth = As 8-bit
    • Remap to game palette = False
    • Transparent color = Leave as is.
  3. Testing sprite in game. This is bit tricky because AGS does not let to read pixels from non-dynamic sprites. Creating a dynamic sprite from imported sprite means there's another step where engine can do wrong. But this is what we have at the moment... I use following test:

    DynamicSprite* dynspr = DynamicSprite.CreateFromExistingSprite(TEST_SPRITE, eColorFmt_8bit);
    DrawingSurface* surf = dynspr.GetDrawingSurface();
    for (int i = 0; i < surf.Width; i++)
    {
    System.Log(eLogInfo, "Color %d,%d=%d", i, 0, surf.GetPixel(i, 0));
    }

The resulting output is:

[Script][Info]: Color 0,0=16
[Script][Info]: Color 1,0=1
[Script][Info]: Color 2,0=2
[Script][Info]: Color 3,0=3
[Script][Info]: Color 4,0=4
[Script][Info]: Color 5,0=5
[Script][Info]: Color 6,0=6
[Script][Info]: Color 7,0=7
[Script][Info]: Color 8,0=8
[Script][Info]: Color 9,0=16

So, here the problem is that AGS replaces index 0 with 16. This is because index 0 is transparency, and ~16 is a substitute for "black" (this is a hardcoded behavior)~ EDIT: scrap that, the behavior is more convoluted than that. This obviously is wrong, as I set "Transparency" import option as "Leave as is". I will investigate when (at which stage) and why it's doing this replacement.

But other color indexes seem to be correct.


Now, in regards to your test8bit.png. When I check the file's properties, I can see that it's a 8-bit PNG. But when importing, I notice that AGS detects it as 32-bit ARGB for some reason. This must be the source of errors, somehow 8-bit PNG is not read as 8-bit. I know that 8-bit PNGs were imported correctly for rooms. Not sure if this is a new recent regression in sprite import, or we forgot to fix PNG loading there; I will investigate this.

ivan-mogilko commented 4 months ago

@ericoporto ok, I pushed a fix to the sprite import, and now your png is loaded correctly as a 8-bit. The test returns this:

[Script][Info]: Color 0,0=-1
Color 146,64=1
Color 288,182=9

-1 (COLOR_TRANSPARENT) is returned by GetPixel in case of a transparent mask color (which is 0 in 8-bit). This is another annoying quirk, but not sure what to do with this at the moment. Curiously, your image did not have color 0 replaced by 16... EDIT: i found that editor is doing something very unexpected when loading a image with palette...

ericoporto commented 4 months ago

Uhm, that is a tricky one, I think the color transparent is alright for bliting/blending and should give the correct result visually, but I think the index zero is expected to be the "wall"/"non-walkable" areas when using MaskPathFinder. My idea would be to secretly replace color transparent by zero for the mask pathfinder somehow.

ivan-mogilko commented 4 months ago

It's not related exclusively to "maskpathfinder", reading pixels from DrawingSurface of the room masks gives the same result, and has same convenience problem.

EDIT: to clarify, the real pixel is 0 in the image, it is GetPixel that returns -1: https://github.com/adventuregamestudio/ags/blob/525b1aa8b3b314c4f1b05ea528fd62e84f6f9d6f/Engine/ac/drawingsurface.cpp#L421-L424

EDIT2: As the sprite itself does not tell whether it's used as a mask or not, there's no way to find this out automatically.

ivan-mogilko commented 4 months ago

Two last commits are unrelated to this new setting, but fix couple of mistakes in importing indexed images. (also cherry picked them to the master branch).

From what I see, the only remaining annoyance here is the aforementioned GetPixel behavior, which returns COLOR_TRANSPARENT constant instead of actual color value for the mask color. I believe that's a matter of DrawingSurface api.

ericoporto commented 4 months ago

I retried this and I think now it's correct