Alpha >1 || Alpha < 0 = Flicker on Android

Monkey Forums/Monkey Bug Reports/Alpha >1 || Alpha < 0 = Flicker on Android

therevills(Posted 2014) [#1]
I've noticed that if you set alpha to be more than 1 or less than 0 it causes images to flicker on Android.

Could we add some automatic bounding for alpha?


marksibly(Posted 2014) [#2]
OK, this will introduce a bit of overhead though. Guess it depends how 'low level' you consider Mojo.

Colors too? Again, a little more overhead...


therevills(Posted 2014) [#3]
OK, this will introduce a bit of overhead though. Guess it depends how 'low level' you consider Mojo.

Would a simple "if" statement add too much overhead?

Colors too?

I haven't tested it myself to see what happens, but it might be good idea as the different targets might display them differently.


Samah(Posted 2014) [#4]
setAlpha(val>1?1:val<0?0:val); // where's the overhead?



marksibly(Posted 2014) [#5]
Personally, I don't mind it the way it is - as long as nothing crashes, I'm OK with 'undefined behavior' in the interests of making things go faster. Perhaps I've just been doing a bit too much c++ lately...

> setAlpha(val>1?1:val<0?0:val); // where's the overhead?

This will probably introduce 2 branches internally, and branches are nice to avoid. Clamping color will introduce 6. Probably not a big deal, but if you're doing a ton of multicolored rects or something it could add up.

But I'll add these - I don't think anyone is really using Monkey for it's 'massively optimized' engine!


marksibly(Posted 2014) [#6]
Another approach would be...

Function SetAlpha:Void( alpha:Float )
#If Config="DEBUG"
    If alpha<0 Or alpha>1 Error "Alpha value out of range"
#Endif
...


...which has the advantage of not needlessly impacting the performance of the majority of apps. Of course, it has the disadvantage of not necessarily catching all potential 'alpha out of range' errors, the way a runtime check/clamp would.

Thoughts?


therevills(Posted 2014) [#7]
At the moment I am having to add "If" statements just before I set an alpha variable, I'm hoping I dont have to do that in the future:
Field alpha:Float = 0

Method Update:Void()
    If alpha < 1
         alpha += 0.01 * Delta
         If alpha > 1 Then alpha = 1 ' unneeded if SetAlpha is clamped
    Else
         alpha = 1
    End
End

Method Render:Void()
    SetAlpha(alpha)
    DrawRect(100,100,100,100)
End

I would prefer the clamping, it would make it safer for all targets.


Fred(Posted 2014) [#8]
I disagree with that choice. It's a user decision to clamp values for alpha and color. I discovered yesterday that glfw handles negative colors. Ok behavior is not consistent over plateformes (for instance color values are clamped with html5). But someone could want to use this kind of "feature" on a specific platform and it's useful to see if your code does what it should. Tests are easy to add on the user side, what do you if you don't want them ?


therevills(Posted 2014) [#9]
What about a preprocessor then? To clamp or not to clamp....


ziggy(Posted 2014) [#10]
I think the original suggestion for Mark is the most sensible one. I'm already using in GuiColor in JungleGui for the same reason. I see it like an array out of bounds check. It's usually safe enough to remove it on release builds


therevills(Posted 2014) [#11]
But that means you have to manual check everywhere in your code when you use SetAlpha/SetColor... which I am trying to avoid.

And it isn't safe for release builds... if you draw something higher than alpha 1 on Android it doesn't show. (With in array out of bounds, it still can crash your release builds).

The whole point of Monkey was that I can create Monkey code and it looks and runs the same on any of the targets it supports, at the moment it doesn't do this (for alpha).


ziggy(Posted 2014) [#12]
My approach is a bit higher level, so it does not need to check for valid values every time but I think Mark was talking about introducing the bounds check in mojo


therevills(Posted 2014) [#13]
My approach is a bit higher level

Sorry could you elaborate?

Is your GuiColor method something like this:
Method GuiColor:Void(r:Int, g:Int, b:Int)
    if r < 0 then r = 0
    if r > 255 then r = 255
    if g < 0 then g = 0
    if g > 255 then g = 255
    if b < 0 then b = 0
    if b > 255 then b = 255
    SetColor(r, g, b)
End

[edit]
Just re-download JungleGUI... ah, GuiColor is an actual class... its an okay approach but I do think it is better within MonkeyX itself.


ziggy(Posted 2014) [#14]
Yes, the idea is that you only need to check on color creation but not everytime a color is used. If you leave this to a class, you can have as many pre-checked colors as you may need, so this checks can be reduced a lot while security is kept. As you know more than well, Mojo is a bit too low level sometimes to be used directly without an actual framework on top of it.


marksibly(Posted 2014) [#15]
> But that means you have to manual check everywhere in your code when you use SetAlpha/SetColor... which I am trying to avoid.

No it doesn't - you only need to manually check if alpha/color can potentially be out of range, and for a lot of code, that's 'never'. For starters, only in situations where you have modified alpha do you need to worry about it. And even then, in most cases you should probably clamp regardless - just letting alpha+=blah style code 'run away' is probably not a good idea as it could lead to FP instability. With a little effort, your example code above could be rewritten so that the alpha>1 check is only performed once and alpha is properly clamped for rendering.

This is kind of one of those parameter checking issues that balances efficiency vs convenience - ie: it's generally more convenient to perform parameter checks at a lower level, but more efficient to perform them at a higher level. Higher level code 'knows' more about what's going on, and can eliminate the need to perform many parameter checks. Adding alpha/color clamping mode to mojo would be convenient, but it would also impact the efficiency of the many apps that don't need/use it. Not majorly perhaps, but it's still worth considering IMO.

Also, Monkey is, for the most part, designed around 'static', debug mode error checking - ie: the graphics API just runs completely free of error checks in release mode - ditto with null objects, array bounds etc. So I think the #If Debug solution above is, if nothing else, more 'Monkey'-like!


therevills(Posted 2014) [#16]
The sample code is a small section of a major project, I call SetAlpha so many times and in multiple places it has become a pain to add the "safety" checks around each one... and when I was testing in HTML5 it works fine, but then checking Android I find all these flickering images in my game.

Again its these small things which could put off new coders, it didnt take me long to find out why the images were flickering but a noobie might start pulling hairs out.

The outcome of this is that the game runs differently on differently targets...


marksibly(Posted 2014) [#17]
Well, the #If Debug check would certainly help find where alpha is flickering >1. And IMO, also solves the 'runs differently on different targets' and 'confuses newcomers' issues, as it basically says alpha values<0 and >1 are illegal - currently, they're really 'undefined'.

But even if mojo did have alpha clamping, wouldn't your code still be clamping too, ie: in order to prevent alpha values 'running away'? In which case, it seems wasteful to clamp twice...I can't help but feel your code just needs a bit of friendly rearrangement!


therevills(Posted 2014) [#18]
Well, the #If Debug check would certainly help find where alpha is flickering >1

Sorry Mark I may have misunderstood, please correct me if I am wrong:
Function SetAlpha:Void( alpha:Float )
#If Config="DEBUG"
    If alpha<0 Or alpha>1 Error "Alpha value out of range"
#Endif

This would stop the current application and throw an error with an out of range error when compiling in debug mode. So I would still have to add the clamping myself.

in order to prevent alpha values 'running away'?

It currently does but on the next loop, it sets alpha to 1 if it isnt less than 1... but OnRender could have been called which causes alpha to be more than 1.
If alpha < 1
   alpha += 0.01 * Delta
Else
   alpha = 1
End


I'm more concerned with 'it runs differently on different targets', so your debug check will be okay and will highlight the issue if the developer is compiling only against debug ;)


marksibly(Posted 2014) [#19]
> This would stop the current application and throw an error with an out of range error when compiling in debug mode.

Yes, exactly. It effectively makes alpha <0 or >1 values 'illegal' instead of just 'undefined'.

> So I would still have to add the clamping myself.

Yes.

> It currently does but on the next loop, it sets alpha to 1 if it isn't less than 1...

Simplest approach is probably just...

alpha=Clamp( alpha+.01*Delta,0.0,1.0 )


I do kinda see your point here though - your code looks like it *should* work fine, and alpha should never be >1 - but of course, due to the delights of FP accuracy, this isn't always the case. I don't necessarily think this sort of thing should be hidden from coders, but I'm still officially 'undecided' about the whole issue - would be nice to get more feedback/opinions...?


therevills(Posted 2014) [#20]
Ha ha - I didn't know that Monkey had a built in Clamp function(s)...

The example I've given is a super simple one, in my current game I am doing extra functions within the 'IF' block not just incrementing alpha.

For now, I'll just go thru my code and wrap my Mojo SetAlpha calls with a Diddy SetAlpha which clamps the alpha, not what I really wanted to do...

Thanks for looking anyway.

[edit]
Or maybe fix it per target, for example mojo.android.java:
	int SetAlpha( float alpha ){
	this.alpha=alpha;
	int a=(int)(alpha*255);
	if (a>255) a = 255;
	colorARGB=(a<<24) | ((int)(b*alpha)<<16) | ((int)(g*alpha)<<8) | (int)(r*alpha);
	return 0;
}


HTML5 works because of the spec:
The value must be in the range from 0.0 (fully transparent) to 1.0 (no additional transparency). If an attempt is made to set the attribute to a value outside this range, including Infinity and Not-a-Number (NaN) values, the attribute must retain its previous value.

http://www.w3.org/html/wg/drafts/2dcontext/html5_canvas_CR/#dom-context-2d-globalalpha


marksibly(Posted 2014) [#21]
> HTML5 works because of the spec:

So html5 isn't clamping then, it's totally ignoring values outside of range.

This is perfectly acceptable behavior too - in fact, IMO it makes more sense than clamping. Alpha values <0 or >1 don't really mean anything, and deciding to clamp them is pretty arbitrary - we could clamp, or take the fractional value (wrap) or generate an error or ignore, etc. If we're not gonna generate an error, I'd probably opt for ignore.

Also, had a play with html5 alpha today and on my Chome at least, it didn't handle alpha <0 at all well!


therevills(Posted 2014) [#22]
I'd probably opt for ignore.

Sounds good, as long as its consist thru all targets :)

Also, had a play with html5 alpha today and on my Chome at least, it didn't handle alpha <0 at all well!

Seems to work here... I played with the w3schools example:
http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_canvas_globalalpha
ctx.globalAlpha=0.2;
ctx.fillStyle="red";
ctx.fillRect(20,20,75,50);
ctx.globalAlpha=-0.5;
ctx.fillStyle="blue"; 
ctx.fillRect(50,50,75,50); 
ctx.globalAlpha=.75;
ctx.fillStyle="green"; 
ctx.fillRect(80,80,75,50);



Fred(Posted 2014) [#23]
I understand the need of consistent behavior over platforms, especially when it leads to the android flickering.
So I prefer the #if CONFIG="debug" then Error, that tells you the value is wrong instead of saying nothing. It will be very more helpful to debug/understand your code, otherwise you can carry the wrong value over other parts of your code risking a bug somewhere else without getting why (as the display is correct).
If you know your code send a wrong value to a function, you definitly need a clamp test somewhere, and as Mark said it's better to manage your values at a higher level instead of relying to a low level check, especially if the test code cannot be more efficient in the low level. If I know my code is always sending good values to SelAlpha I wouldn't appreciate to discover that the function tests it all the time.
By the way I appreciate Monkey a lot for it's "fewer possible code" approach, keep it this way. I'm about to write about this in Roadmap thread.


ziggy(Posted 2014) [#24]
So I prefer the #if CONFIG="debug" then Error, that tells you the value is wrong instead of saying nothing. It will be very more helpful to debug/understand your code, otherwise you can carry the wrong value over other parts of your code risking a bug somewhere else without getting why (as the display is correct).
Agree. Lots of time I forget that Mojo alpha is a simple float factor, while r g and b are integers, and I usually set Alpha = 255 and don't realise the error until it's too late (as html5 just swallows it)


Nobuyuki(Posted 2014) [#25]
I heavily disagree with clamping alpha and color values to prevent undefined behavior. For one thing, usually that behavior is usually apparent (though it doens't crash!) and shows you where you need to be clamping your own values. Secondly, I also agree with the notion that auto-clamping will incur a speed penalty, which isn't an acceptable trade-off considering many people here already clamp our alpha values when/where necessary. It's just bad practice to not clamp the values yourself if they're going to fly out of the 0-1 range, and others shouldn't have to pay the performance penalty for people who want to do this without thinking about it...


therevills(Posted 2014) [#26]
I heavily disagree with clamping alpha and color values to prevent undefined behavior.

I totally disagree, we are talking about undefined behavior therefore it should be fixed and clamped or a warning given. SetAlpha has valid values of 0.0 to 1.0, SetColor has valid values of 0 to 255, if you go beyond these the API should warn or control the values.

I also agree with the notion that auto-clamping will incur a speed penalty,

A very very small penalty which would stop a lot of weird effects.

many people

Source of "many" people please... You have stats and facts?


ziggy(Posted 2014) [#27]
I totally disagree, we are talking about undefined behavior therefore it should be fixed and clamped or a warning given. SetAlpha has valid values of 0.0 to 1.0, SetColor has valid values of 0 to 255, if you go beyond these the API should warn or control the values.
Re-reading all the posts, In my honest opinion, I think the debug-time error Mark was proposing is the best approach.