Bug: Tinting inconsistencies between DirectDraw and D3D [FIXED]

[Calin Leafshade]

I know this was addressed a while ago but it seems its still not 100% fixed.

I've built a little lightmap tinting engine and it works fine in DD mode but in Dx9 mode it seems to be rather unhappy with certain values..

for example... 232 184 80 RGB seems to tint a kind of purpley colour but 240 184 87 RGB tints an orangey colour (which is correct).

The colours are pretty much identical to the eye so surely they should tint the same?

Edit by strazer:

Fixed in AGS v3.2.1:  - Fixed D3D tints not working properly (3.2 regression) (Nefasto)

[Pumaman]

What are you setting a tint on? A character, or region, or an ambient tint?

[Calin Leafshade]

A character.

Basically the character is tinted to the value of a pixel at the characters x, y of a dynamic sprite.

So the tinting is done every game loop

At certain positions however the character seems to jump to a wildly different shade to the pixels surrounding it.

This only happens in D3D not DD

[Pumaman]

Last time we looked at this, I'm sure the conclusion was that the D3D tint operates correctly, but the DD tint has a bug that doesn't always tint properly. As far as I know the D3D tint should be correct.

[Calin Leafshade]

I'm fairly convinced *something* is wrong with the d3d tinting in this case since the DD tinting works exactly as expected.

Would you like me to upload a little example illustrating the issue?

[Pumaman]

for example... 232 184 80 RGB seems to tint a kind of purpley colour but 240 184 87 RGB tints an orangey colour (which is correct).

Thanks for the example, I've reproduced this with the following test case:

   player.Tint(232, 184, 80, 100,100);  <-- reddish tint
   player.Tint(232, 184, 81, 100,100);  <-- orangeish tint

It's probably a problem with the D3D pixel shader. I've uploaded the shader code here:
http://www.adventuregamestudio.co.uk/tintshaderLegacy.txt

can anyone see a problem with the code?

[Calin Leafshade]

I've debugged the shader with RenderMonkey and I can't see any problems with those specific values.

However, the shader takes HSV values and not RGB values. So I bet the error is in the conversion between the two colour spaces which happens inside AGS.

Can we see your conversion algorithm?

EDIT: Actually the problem doesnt seem to exist on my desktop (with an ATI card) I'll try testing the shader on my laptop (with an nVidia card)

EDIT AGAIN: Checked on laptop using independently calculated HSV values for CJs example and they seem fine. The difference between the two values is +- 0.4% of saturation which is barely noticeable but the shader renders them perfectly as far as i can tell.

[Pumaman]

The RGB->HSV conversion in the engine uses the standard Allegro rgb_to_hsv function. I've checked the HSV values being sent into the shader, in the example I tried they were:

RGB(232,134,81) -> HSV(0.1137, 0.65, 0.91)
RGB(232,134,80) -> HSV(0.1140, 0.65, 0.91)

which seems quite reasonable... unless there's some sort of problem with the way one of the values is getting passed from the AGS code into the shader. Not sure how we would debug that?

[Yeppoh]

I would think the floats are either rounded up or down in some cases with DX9.

I would suggest to find a way to just use int values instead of float ones in the hsv_to_rgb function so that you get :

RGB(232,134,81) -> HSV(1137, 6500, 9100)
RGB(232,134,80) -> HSV(1140, 6500, 9100)

And then convert them back into RGB after all the needed blendings.

But after giving a look at the shader txt you posted, that would take lots of work.

[Calin Leafshade]

you cant really use ints like that very efficiently in HLSL.

I've done some tests with a white square and neither value tints particularly accurately.

however it seems that the hue value jumps from 0.114 to 0.947 which doesnt sound like a rounding error to me since CJ confirmed that the values were sent to the shader correctly.

Why is a shader used for this? Surely using a shader stops old graphics cards from using the dx9 mode?
Why can't a colour value just be passed to the d3d matrix?

[Yeppoh]

I see.

Then I have another idea. In your shader txt you posted, in the hsv_to_rgb function, would it be better to write the if conditions like this :

Code: ags

if     (var_i >= 0.0 && var_i < 1.0)           { RGB = float4(HSV.z, var_3, var_1, 1.0); }
       else if (var_i >= 1.0 && var_i < 2.0) { RGB = float4(var_2, HSV.z, var_1, 1.0); }
       else if (var_i >= 2.0 && var_i < 3.0) { RGB = float4(var_1, HSV.z, var_3, 1.0); }
       else if (var_i >= 3.0 && var_i < 4.0) { RGB = float4(var_1, var_2, HSV.z, 1.0); }
       else if (var_i >= 4.0 && var_i < 5.0) { RGB = float4(var_3, var_1, HSV.z, 1.0); }
       else                                                  { RGB = float4(HSV.z, var_1, var_2, 1.0); }

Since the var_i is a float ?

[Kweepa]

I am very confused.
Why are you converting to HSV at the engine level, and then in the shader, converting back to RGB?
Is it possible to keep everything in RGB space?

I see a couple of potential problems...

(1) You calculate something you call a luminance, then feed it as the value parameter to hsv_to_rgb. It appears that you are confusing HSL and HSV. It could just be that you are using the wrong names for things, but it's something to double check. Note that S calculated from a RGB->HSV calculation is a different thing to S calculated from a RGB->HSL calculation.

(2) The calculation of luminance is based on the max of R, G, B. That sounds like the value rather than the luminance, so all may be good there after all...

(3) To adjust for the transparency, you subtract (1-transp). I would expect instead you to multiply by (1-transp). I can't see that being the cause of your problems, but it's something to check.

Your hsv_to_rgb looks ok.

What does the Allegro side look like?

[Pumaman]

I would think the floats are either rounded up or down in some cases with DX9.

As Calin says, the fault here is too big to be a simple rounding error ... something else must be going wrong, but I'm not sure what it is.

Why is a shader used for this? Surely using a shader stops old graphics cards from using the dx9 mode?
Why can't a colour value just be passed to the d3d matrix?

As far as I know, a shader is the only way to achieve this type of tint. If you can get the same result without using a shader, then that would be great, but I don't think it's possible.

Then I have another idea. In your shader txt you posted, in the hsv_to_rgb function, would it be better to write the if conditions like this :

I think it's ok, because of this:
float var_i = floor(var_h);
which ensures var_i is rounded down to an integer

I am very confused.
Why are you converting to HSV at the engine level, and then in the shader, converting back to RGB?
Is it possible to keep everything in RGB space?

The way that the AGS tint has always worked is that it takes the H and S of the tint colour, and combines them with the V of the pixel. In order to be consistent with the DX5 tint, the D3D shader therefore needs to do the same thing.

(1) You calculate something you call a luminance, then feed it as the value parameter to hsv_to_rgb. It appears that you are confusing HSL and HSV. It could just be that you are using the wrong names for things, but it's something to double check. Note that S calculated from a RGB->HSV calculation is a different thing to S calculated from a RGB->HSL calculation.

Good point, it is HSV, the GetLuminance function is badly named and should be GetValue really.

(3) To adjust for the transparency, you subtract (1-transp). I would expect instead you to multiply by (1-transp). I can't see that being the cause of your problems, but it's something to check.

I think this is ok, as transparency is actually a float4 where transparency[0] is the transparency, and transparency[1] is the light level; it multiples by (1-lightlevel) to darken the image if applicable.

What does the Allegro side look like?

I think the Allegro code is ok, because the HSV result that it returns seems reasonable in the example I posted above.

This is quite mysterious, Calin how did you discover that 0.947 is being passed in to the shader?

[Shane 'ProgZmax' Stevens]

I notice in both cases it's the Blue channel that seems to be causing the trouble.  Is there any chance that the way you've handled the blue channel internally with the 0-31 support for legacy 'special' colors is interfering somehow?  This is a rather wild guess based on what little I could learn from how the editor handled the the colors, so I may be way off.  In any case, it may be worth checking to see if the red and green channels suffer equal problems with 'minor' variances like 120,80,100 vs 120,84,100 and so on.

[Yeppoh]

Then I have another idea. In your shader txt you posted, in the hsv_to_rgb function, would it be better to write the if conditions like this :

I think it's ok, because of this:
float var_i = floor(var_h);
which ensures var_i is rounded down to an integer

I also thought it was ok.
However it's how the output of the colours when the bug appears, that I consider the problem maybe comes around there.
I noted down in a table many values using, like Calin did, a white square and checked the output colour with an external  software, with the hope to find a function for predicting when it occurs with differenciation.

For a more concrete example :
For (255, 245,  0) I get (255, 0, 10);
For (255, 239,  0) --> (255, 0, 16);
For (255, 128, 22) --> (255, 22, 149);
For (255, 156, 50) --> (255, 50, 149);
etc...

Then when I checked the shader txt, I saw that the last line of the conditional

Code: ags

...
else { RGB = float4(HSV.z, var_1, var_2, 1.0); }
...

seemed to fit the wrong values above, as if this line was processed instead of the right one in the conditional.
Thus my thinking that the int that the floor function returned may be badly interpreted in some weird cases and unknown reasons by the float var_i. So I came up with that correction just in case it would be the problem.

[Pumaman]

Excellent spot, Nefasto!
I've just made the change you suggested, and the problem seems to be fixed.

Can you guys please try this patched ACWIN:
http://www.adventuregamestudio.co.uk/acwin.zip
and see if it resolves the problem?

[Yeppoh]

Yep! It works like a charm on my side now.

Thanks for testing my suggestion!

[subspark]

Brilliantly resolved guys! Big cheers,
Sparky! 

[Pumaman]

Excellent! Glad to hear it, I'll include this in the next release.