Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AGS 4: support Alpha in Color parameter #2525

Open
6 tasks
ivan-mogilko opened this issue Sep 1, 2024 · 13 comments
Open
6 tasks

AGS 4: support Alpha in Color parameter #2525

ivan-mogilko opened this issue Sep 1, 2024 · 13 comments

Comments

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Sep 1, 2024

This is written after #1514 and #1980.

Now when AGS supports true 32-bit colors for drawing in game, we may consider supporting Alpha value as well.
In terms of storing Alpha component in a color number, this is already supported by the field format, but there are several issues to resolve before this may be actively used.

The subtasks follow:

  • 1. Expand Game.GetColorFromRGB to also have "alpha" parameter. For backwards compatibility, and also because it's less common, add it as a last optional parameter with default value of 255 (full alpha).
  • 2. Make sure that all the drawing operations in Allegro support alpha blending (not just blit, bit also draw line etc). I've never investigated that, but this may not be trivial, depending on how they work. IF Allegro does not support that, then we might have to implement our own, by cloning standard allegro functions, and more or less passing any pixel assignment in them through a blender (pixel function). These functions will be called if alpha if anything but 0 or 255.
    In case alpha is 255 the standard functions are called;
    In case alpha is 0, nothing is drawn and whole operation may be skipped.
  • 3. If alpha is supported, then all the object Color properties must respect alpha component as well. Such as: GUI colors, speech color, and so forth.
  • 4. BTW, there will be a side effect of this, that color 0 which is currently used in some cases meaning "transparent" (even in 32-bit games) will represent a 0,0,0,0 ARGB (so color with alpha == 0), and therefore it will be possible to assign true-black color to GUI background and such using any non-zero alpha. But this has to be double checked.
  • 5. Editor: there's a issue of the color picker. The common color picker dialog which we currently use does not let input alpha. Maybe there's a mode in which it does, then it will simply have to be enabled. But if not, then we'll require a different dialog.
    In short term we may leave things as is, but then you won't be able to set alpha in the editor other than by editing a Color value by hand (and choosing a different color in the picker dialog will reset alpha to 255).
  • 6. Editor: a minor issue is that since all color properties are ints, that is - signed ints, having alpha in them will make them go negative. That's not an issue in UI anymore, since I changed all Color properties to be displayed as RGB sequence, but they will be written as negative in the Game.agf xml. What can be done about it? We may change all of these properties to uint; but these properties are in AGS.Types, so I wonder if that will affect editor plugins. I am not versed enough in how C# interfaces work to predict the consequence of this change. Another way is to serializes these as hex. We'd probably need to add a new attribute "SerializeAsHex" for these properties.

Is there anything else that comes to mind that might require attention?

@ivan-mogilko ivan-mogilko added this to the 4.0.0 (preliminary) milestone Sep 1, 2024
@ericoporto
Copy link
Member

ericoporto commented Sep 1, 2024

we don't have uints in ags script itself, what's the idea of colors there?

Uhm, the text functions use blit so if the color alpha isn't 0 I think they are going to be weird. (They don't blend , this means they will replace the alpha of whatever was meant to be below the text)

Other issue may be anything that has color and transparency.

In general I have a bad feeling for alpha usage in color fields. Also about drawing remember that bitmaps needs to be fixed currently for the color leak issue when using linear blending instead of nearest neighbor in the bitmap to video functions.

There's also the pixelperfect click detection

My gut feeling is to not support alpha as color.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 1, 2024

Having alpha in a drawing color is the same as having alpha in imported sprites. All the logical and technical difficulties are shared between these 2 cases. When a final image is used by an object, or drawn elsewhere, there's no difference in whether it was created by importing pixels from a file, or by drawing pixels following a script command.

we don't have uints in ags script itself, what's the idea of colors there?

This does not matter, as colors are encoded as ARGBs, and should not be viewed as numbers, but as something that has a sequence of bytes combined with bitwise operations.

@ericoporto
Copy link
Member

ericoporto commented Sep 1, 2024

remember we do a post processing currently of alpha on import in the editor! So if you read alpha it won't match the color that it was when you saved on the image editor. Plus a lot of image editors DO NOT save non black full alpha - what I mean is if you draw 0x00RRGGBB it will be then saved as 0x00000000 by a lot of image editors. We may need to notedown this somewhere in the manual because I understand the image preprocessing in the editor is going to be removed.

int bitmapMaskColor = toimp->GetMaskColor();
int replaceWithCol = 16;
if (toimp->GetColorDepth() > 8)
{
if (importedColourDepth == 8)
replaceWithCol = makecol_depth(toimp->GetColorDepth(), itspal[0].r, itspal[0].g, itspal[0].b);
else
replaceWithCol = 0;
}
// swap all transparent pixels with index 0 pixels
for (int tt=0;tt<toimp->GetWidth();tt++) {
for (int uu=0;uu<toimp->GetHeight();uu++) {
if (toimp->GetPixel(tt,uu)==transcol)
toimp->PutPixel(tt,uu, bitmapMaskColor);
else if (toimp->GetPixel(tt,uu) == bitmapMaskColor)
toimp->PutPixel(tt,uu, replaceWithCol);
}
}
}

my main concern :

if (is_color_mask<T>(src_color))
{
// set to transparent, but use the colour from the neighbouring
// pixel to stop the linear filter doing colored outlines
T red = 0, green = 0, blue = 0, divisor = 0;
if (x > 0)
get_pixel_if_not_transparent<T>(&srcData[-1], &red, &green, &blue, &divisor);
if (x < t_width - 1)
get_pixel_if_not_transparent<T>(&srcData[1], &red, &green, &blue, &divisor);
if (y > 0)
get_pixel_if_not_transparent<T>(
(const T *) &scanline_before[(x + t_x) * src_bpp], &red, &green,
&blue, &divisor);
if (y < t_height - 1)
get_pixel_if_not_transparent<T>(
(const T *) &scanline_after[(x + t_x) * src_bpp], &red, &green,
&blue, &divisor);
if (divisor > 0)
idst[x] = VMEMCOLOR_RGBA(red / divisor, green / divisor, blue / divisor, 0);
else
idst[x] = 0;
lastPixelWasTransparent = true;
}

this is_mask check is about to be more expensive, this is the most expensive function in the bitmap to video memory.

Other than this, all the pixelperfect tests that currently simply compare with the mask color - this is for clicking on a character or an object.

plus blitting text where the color is alpha is going to puncture holes in the dialog boxes - they would need to blit to make the border in a separate bitmap and then later blended.

This does not matter, as colors are encoded as ARGBs

sure you need to do something like FFacolor now because you need the alpha to be 255. this means we are always using a negative int.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 1, 2024

That alpha post-processing replaces colors with alpha == 0 with a mask color (00FF00FF in 32-bit). It does not affect those pixels with non-zero alpha, they will keep their alpha value.

What I meant is, AGS already may have sprites with pixels where alpha is between 0 and 255, which come by importing sprites. These sprites already affect pixel-perfect detection logic, for example. Having those alpha values put by a drawing operation in script will not add anything new here.

Similarly, these sprites with varied alpha already pass through bitmap->texture conversion. Having pixels drawn with alpha won't add anything new here either.

The only thing which will affect logic here are colors with 0 alpha. As you rightly mentioned, currently they are replaced by the "transparent color" constant. I did not yet decide what should happen with the "transparent color" constant, this is something to think well about, keeping consequences in mind. But in the worst case, in a short term, we may keep the rule that any drawing color with alpha 0 becomes a "mask color". Sure that will be strange (to read these pixels back), but will keep things consistent so long as this post-processing exists in sprite load.

plus blitting text where the color is alpha is going to puncture holes in the dialog boxes - they would need to blit to make the border in a separate bitmap and then later blended.

Yes, I am aware of that. That's why I mentioned that possibly we'll have to have 2 sets of functions: for opaque drawing color, and non-opaque drawing color, where the first uses fast blitting and second uses an extra processing. This way common operations will be kept as fast as before.

sure you need to do something like FFacolor now because you need the alpha to be 255. this means we are always using a negative int.

That is correct. Do you see a problem with this?

EDIT:
From my understanding, the whole point of having drawing color with alpha component is to make drawing fully consistent with importing the sprite. If an image may be imported having varied alpha values in its pixels, then why not allow to make similar image in script?

EDIT2:
Oh right, another use for having ARGB color properties is potentially for defining shader effects.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 1, 2024

my main concern :
this is_mask check is about to be more expensive, this is the most expensive function in the bitmap to video memory.

Could you elaborate, why would it be more expensive, and in which circumstances?
I did not think about this particular function yet, but in a hypothetical case when the "mask color" is gone, the equality check
return c == MASK_COLOR_32
will likely be replaced by a bitwise AND, something like:
return !static_cast<bool>(c & 0xFF000000)

@ericoporto
Copy link
Member

I think overall the idea of keeping the mask color is a good one, simply because most image drawing software do not keep the colors when drawing with zero alpha and write with 0 instead to all colors

we may keep the rule that any drawing color with alpha 0 becomes a "mask color". Sure that will be strange (to read these pixels back),

I believe this will save us from debugging lots of user images where their software is not saving what is in memory correctly. Plus if we are going to have to do the fix for linear rendering anyway for our own drawing surfaces, there isn't much to gain from importing the images from users and keeping their alpha untouched (supposing someone uses a software that allows to correctly prepare the 0 alpha colors correctly for linear rendering).

@AlanDrake
Copy link
Contributor

AlanDrake commented Sep 2, 2024

  1. Game.GetColorFromRGB should stay for RGB, instead we should create a new Game.GetColorFromRGBA which is clear in its intent.

  2. The native color picker doesn't have alpha support. We'll need a custom component.

  3. Color values being negative should not be a problem. Storing them in hexadecimal format is probably better, if there are no major obstacles in doing so.

As for scripting, what is going to happen with .XXXColor properties?

  • Do we break compat and use the real value? Ancillary methods like .XXXColorRGB/.XXXColor24 could be added by a module for convenience. I don't think it would be too much work to upgrade a project.
  • Will they remain as 24bit for compatibility, and perhaps add a .XXXColorARGB that exposes the full value?
  • Maybe a general setting to enable/disable ARGB behavior? Though, that would be annoying to check on every place, and it would litter the code for the sake of backward compat, which ags4 is meant to remove.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 2, 2024

As for scripting, what is going to happen with .XXXColor properties?

My first thought was that these will need to be updated on older project / game load, but that is it. The colors should not be set directly at all, but using Game.GetColorFromRGB(A), which solves the compatibility issues in script (I guess?).
Do you see any possible troubles with that?

BTW, to clarify, all of these properties are already updated on project / game load after changing from 16-bit to 32-bit values.

Then, I'd even replace these properties with ColorType struct (it exists in the script api already) which has RGB fields inside, but AGS cannot return structs from a function, so sadly obj.Color = Game.GetColorFromRGB() won't work, and neither would passing these into other functions as arguments.

@ericoporto
Copy link
Member

ericoporto commented Sep 2, 2024

The colors should not be set directly at all, but using Game.GetColorFromRGB(A)

Why not setting them directly? Couldn't they be set with hex like 0xFFRRGGBB ?

I am mostly worried because Everytime I need to call the script API it takes a lot of time and mostly of the optimized stuff I do in AGS Script is basically doing as few call to the engine as possible and this will make it obligatory to use the engine script api for something that was possible to do without going there at all. Calling functions from the script API is SUPER slow.

obj.Color = Game.GetColorFromRGB()

I think if it needs to be a managed struct these should be something like obj.Color.RGB = 0xRRGGBB.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 2, 2024

Why not setting them directly? Couldn't they be set with hex like 0xFFRRGGBB ?

Mmm, yes, that is true.

For the most of the common users, and simple cases, GetColorFromRGB would be more convenient, as that does not require to worry about how values are formed.

Setting encoded values directly will be faster, but requires to comply to the current form.
So, unfortunately, if using these values, they, or their generation in script, will have to be adjusted depending on the current format, when upgrading a project.

So, now I see that the problem of backwards compatibility can be real in this regard.

EDIT:
Just a quick thought, in theory a game could have a "default alpha" value setting, which is simply ORed with the color value passed from a script (color = color | defaultAlpha;). This "default alpha" is 0xFF for the "backwards compatible" mode and 0x00 for the new mode.
This could be even a drawing surface property.

@AlanDrake
Copy link
Contributor

But AGS4 is meant to drop retrocompat. I'd rather we did not keep special cases, unless for the most dire necessity.

Even in the worst case, a user would spend no more than a couple of days fixing their color assignment instances.

Would it be possible to have alternative assignments like erico suggested? Like obj.Color.RGB = 0xRRGGBB while still letting obj.Color = 0xFFRRGGBB work for direct color?

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 2, 2024

Any kind of multiple assignment would require to replace int XColor properties with a struct that has multiple properties. Along with the change of API, that will make such assignment slower (I mention this as performance concern was noted above).

In this context, an idea of having separate property for "color with alpha" does not look so bad...

Another approach could be in supporting parameterized macros, in which case we could replace GetColorFromRGB with a macro too, and have macros like GetOpaqueColor(r,g,b) which would add 0xFF alpha.

But I don't have a defined opinion on this atm.

There's an alternative posted as #1514 originally, where Alpha itself is a standalone property for drawing color in DrawingSurface.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Sep 6, 2024

I see now that I opened this ticket too early, without thinking it all through from the design perspective.

There are cases where having alpha in a color will either duplicate existing feature, or may not be enough to achieve all wanted effects.

  1. For objects that may have either a color or a graphic, such as GUI, there's already Transparency property that does exactly what alpha component would do in a Color property. Which means that use of alpha in the Color there is mostly useless (at least without changing the meaning of existing properties).

  2. For DrawingSurface, there are 2 separate operations that may be wanted:

  • write alpha into surface pixels (copy color)
  • draw something with alpha (blend color)

Unfortunately, I did not ask @fernewelten which exactly effect of these two did he have in mind when writing #1514.
But having an alpha component in DrawingColor will likely achieve the second effect, while the first effect will still be not possible to achieve. For the first effect we'd need a separate drawing parameter, something that selects between Copying pixels and Blending pixels.

I need to think more about all this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants