U4 doesn't render transparency correctly in "colourmap" PNGs

[prl]

Which would mean that the skin needs to be changed to use alphablending.

In some cases. There are a reasonable number of pixmaps in the built-in skins that should be rendered either with alphatest="off" or alphatest="on". Of those, most are presently alphatest="on", but many (perhaps all) should be alphatest='off".

The main issue with the changes is that the recent changes to the bitmap code alphatest="on" threshold changed the threshold for 8-bit colourmap pixmaps, but not for RGBA pixmaps (which already had the alpha != 0 test for opacity).

However, in general, the RGBA pixmaps are the ones where the test should be alphatest='off", and where changing them to alphatest="blend" would give exactly the effect that this topic was initially complaining about.

Note that 8-bit colourmap pixmaps outnumber RGBA pixmals by more than 5:1 in easy-skin-aus-hd and by about 4:1 in Full-Metal-Wizard.

My suggestion is that in gdi/gpixmap.cpp, ALPHA_TEST_MASK should be changed from 0xFF000000 to 0x80000000.

It looks as though the easy-skin-aus-hd pixmaps were designed to accommodate systems that don't support alphatest="blend".

[peteru]

My suggestion is that in gdi/gpixmap.cpp, ALPHA_TEST_MASK should be changed from 0xFF000000 to 0x80000000.

No. That would just be propagating incorrect behaviour. The correct fix (even though it's more work) is to fix the alpha/mask on the PNGs and use the alphatest option that is appropriate for the file.

[IanB]

Hints to clearer thinking :-

Think of AlphaTest 0x80000000 as a poor mans if (alpha >= 128) copy pixel as opaque


Also note the AlphaBlend case leaks slightly as (255*255)>>8 = 254

[prl]

My suggestion is that in gdi/gpixmap.cpp, ALPHA_TEST_MASK should be changed from 0xFF000000 to 0x80000000.

No. That would just be propagating incorrect behaviour.

It's not clear to me why the test alpha > 0 is correct and alpha >= 128 is incorrect. Testing alpha >= 128 seems completely reasonable to me.

The correct fix (even though it's more work) is to fix the alpha/mask on the PNGs and use the alphatest option that is appropriate for the file.

Yes, why not? There are only about 1000 of them. Some won't need an alphamask tweak, but a decision will have to be made on each one, and a fairly large proportion will need a fix.

[prl]

...
Also note the AlphaBlend case leaks slightly as (255*255)>>8 = 254

That's probably a speed hack. Division by 255 would be correct, but would put a quite expensive operation in the inner loop of the blend. There are perhaps similar speed considerations for why the full Porter and Duff equations aren't used for the colour blending.

[IanSav]

Hi,

According to a post on the OpenPLi forum Xtrend (who I believe designed the U4) will be updating their drivers to address the hardware blend issues. The thread is https://forums.openpli.org/topic/59164- ... rible-bug/ (gpixmap.cpp, horrible bug...). The particular post of interest is https://forums.openpli.org/topic/59164- ... t&p=871537 (Post #39).

Regards,
Ian.

[prl]

That "horrible bug" isn't in the Beyonwiz code.

Also, I don't think you've accurately described peteru's changes in that discussion. The changes consistently use hardware accelleration for blitAlphaBlend unless FORCE_NO_BLENDING_ACCELERATION is set, and to always use the software implementation for blitAlphaTest.

They also consistently (and IMO incorrectly) use > 0 as the test for opacity for all instances of the code for blitAlphaTest, rather than a mix of > 0 and > 0x80. IMO the test should be uniformly > 0x80.

[IanSav]

Hi Prl,

I am not closely tracking this issue. I just wanted to provide the information should it be helpful to either party.

Regards,
Ian.

[peteru]

It's not clear to me why the test alpha > 0 is correct and alpha >= 128 is incorrect. Testing alpha >= 128 seems completely reasonable to me.

Because it is desirable to perform the keying operation on a single specific value, not at 50% threshold. By choosing a single value at the end of the scale, you preserve as much dynamic range of the remainder of the alpha channel as possible, instead of losing 50%. If you look at compositing several layers, it becomes clear that preserving the alpha can come in useful to blend multiple layers correctly.

[peteru]

I've been thinking about the old alphaTest semantics a bit more and in particular trying to figure out why they would be more desirable.

With the old semantics, you get srcAlpha copied over for anything with the high bit set, otherwise the dstAlpha will be preserved. Except:

• when the source is a 16-bit or 32-bit and destination is 32-bit, in which case the srcAlpha test is for zero.
• when the source and destination are both 8-bit, in which case the pixel is skipped if the source pixel value is zero.

So, besides being inconsistent, you get colour and alpha copied from source for all srcAlpha values from 128-255 and for srcAlpha values between 0-127 the destination pixel is left intact.

I can't think of a good reason for doing that, except for maintaining compatibility. However doing so would be at the expense of predictable behaviour. Simply converting an image from 8-bit to 32-bit (while still preserving all the correct pixel and alpha values) changes the behaviour when using the old semantics.

[prl]

I'm definitely not proposing a return to the old, rather muddled semantics.

For 8-bit colourmaps -> 8-bit colourmap, the test on whether the pixel value is 0 seems to be similar function to the GIF "transparent background pixel", except that in GIF any single colourmap position can be declared transparent, not just 0.

[prl]

... By choosing a single value at the end of the scale, you preserve as much dynamic range of the remainder of the alpha channel as possible, instead of losing 50%. ...

That would read better as "you preserve as much dynamic range of the remainder of the source's alpha channel as possible, while removing almost all the dynamic range of the destination's".