New sprite import keeps alpha channel when converting 32-bit sprites for non-32-bit games

[ghost]

This was found by a user recently when testing 3.5.0 beta.

If you have a 16-bit game, then importing 32-bit sprites will convert them to 16-bit (at least this is what their import property tells). But if the "Import alpha channel" checkbox is set then the sprite will get AlphaChannel = True property anyway. This breaks their drawing logic in the editor (and maybe engine too) making them invisible when assigned to objects in the room.

Not sure what's the correct course for fixing is here. Native code may need to remove alpha channel flag if the sprite is not 32-bit because all drawing logic in AGS depends on it, it seems.

[morganwillcock]

In general there is a conflict between what is stored in the sprite instance in the editor and what actually happened when the sprite was imported. I'll have to double check, but I think I opted to store this value in terms of what was requested by the importer rather than the result of the import - this means that re-importing would maintain the same request of what should be done rather than store the outcome of the previous import attempt.

I don't think it would make a difference for the engine, but I hadn't considered that drawing in the editor would be checking this value.

[ghost]

I also had a thought, maybe there should be separate group of "import properties" and "current properties" (readonly) of sprite? There's already width, height and color depth, perhaps we could add "Has alpha channel" readonly property as well.

[morganwillcock]

I initially thought the same, but if trying to move towards a "compile on usage" model for the spritecache I'm not sure that relying on spritecache state for editor operations would be a good thing in the long term. I think as far as AGS is concerned only 32 bit graphics can have an alpha channel, which I don't think is technically correct, but that is how the internal checks are applied in practice. Perhaps it is better in the long term to just ignore the alpha channel for drawing in the Editor when the graphics are not 32 bit, which I think is inline with how other functions are working and doesn't incur a reliance on spritecache state.

[ghost]

I initially thought the same, but if trying to move towards a "compile on usage" model for the spritecache I'm not sure that relying on spritecache state for editor operations would be a good thing in the long term.

But should not editor know the sprite's current state in order to draw it anyway? Some of the import properties are conditional and do not always reflect actual result. It will have to get actual state somehow, either from native spritecache or else.

The readonly properties I propose are going to reflect current state spritecache. They do not have to be part of game data and may be removed if necessary in the future (although I don't know why).

[morganwillcock]

I only mean that the reliance on the property removes the option to have the spritecache built entirely at compile time, whereas if you extend assumptions that are already made about what can have an alpha channel to the graphics preview, then you don't get the same restriction and you don't get a read/write property as well as a read-only one. I also mention this because, this doesn't sound entirely like an issue caused by the alpha channel setting if the image is rendered entirely invisible, but I'd have to look into how the drawing is done (I'd like to check whether the alpha channel is part of what is requested through the native proxy and whether this results in an empty bitmap).

[ghost]

this doesn't sound entirely like an issue caused by the alpha channel setting if the image is rendered entirely invisible, but I'd have to look into how the drawing is done (I'd like to check whether the alpha channel is part of what is requested through the native proxy and whether this results in an empty bitmap).

There's a branching that depends on it and supposedly the "alpha channel" branch simply does not work if the bitmap is not 32-bit (or destination is not): https://github.com/adventuregamestudio/ags/blob/master/Editor/AGS.Native/agsnative.cpp#L2025

IIRC the engine does this differently, in non-32bit games it strips alpha channel on load and uses simple blitting, which results in sprites drawn but with blackness (or other hue) instead of transparent areas.

[ghost]

I did following experiment: 1) Switched game to 32-bit, and imported a sprite with alpha channel; it ended up 32-bit ofcourse. 2) Switched game to 16-bit, and imported same sprite another time (as a separate instance); it ended up 16-bit but with "alpha channel" flag set.

Then added two objects with these sprites (game was still 16bit at this point). First sprite, 32-bit with alpha channel flag was drawn correctly in both editor and engine (the alpha channel could have been replaced with magic pink along the way, but I did not have time to investigate), second 16-bit was invisible in editor but drawn with blackness instead of transparency in the engine.

You could have 32-bit sprites with alpha channel in 16-bit games before 3.5.0 too, if you switch game's color depth mid-development, but it never set "alpha channel" flag for 16-bit bitmaps, and that's the main difference right now it seems.

[morganwillcock]

in non-32bit games it strips alpha channel on load

Do you mean within the engine when playing the game the alpha channel is stripped, and inside the sprite cache the alpha channel still exists? If it didn't then you wouldn't be able to safely switch between colour depths in the game settings.

it ended up 16-bit but with "alpha channel" flag set

If the answer to the first question is yes, isn't this flag being set to true representing the correct state of the sprite cache?

[ghost]

Do you mean within the engine when playing the game the alpha channel is stripped, and inside the sprite cache the alpha channel still exists?

I maybe confused this with some other case when the sprite actually remains 32-bit in a 16-bit game.

In this case the sprite was saved as 16-bit, yet its "alpha channel" flag is set. This is seen under debugger, and also sprite properties report "16-bit" for the sprite. I think this combination causes the trouble.

[ghost]

I found something else, related.

There's a UpdateSpriteFlags method run after a game was loaded, and it does this: https://github.com/adventuregamestudio/ags/blob/ags3/Editor/AGS.Native/agsnative.cpp#L2124 (the code around changed a bit recently but this particular line is very old)

The problem is that Sprite.AlphaChannel property has changed its meaning. Earlier it referred to an actual sprite's state after import, now it refers to import settings that may or not correspond to final result.

[morganwillcock]

Is there a reason that the bitmap isn't checked on import within the native proxy? i.e. at the moment it looks like it just sets the alpha channel flag with no checks https://github.com/adventuregamestudio/ags/blob/937fd63ae939bd6bfa6599dfccc0ef58acba05b2/Editor/AGS.Native/agsnative.cpp#L2437-L2450 ...couldn't this be checked against bmp->IsAlphaPixelFormat ?

[ghost]

Guess it was just relying on alphaChannel having correct value.

[ghost]

Fixed by #791