[BUG+FIX] Blitting wrong alpha values (Burnings and IImage)

You discovered a bug in the engine, and you are sure that it is not a problem of your code? Just post it in here. Please read the bug posting guidelines first.

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Tue Dec 05, 2017 10:43 pm

@burningreggae - I'd certainly like that new channel. But what would it be named? Maybe the blitter operation could be labeled BLEND_TEXTURE_COLOR_COMBINE_ALPHA.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Tue Dec 05, 2017 10:49 pm

If we make a new pixel blending operation a second function, we could name it "PixelCombine32".
The IImage function to access it could be called "copyToCombineAlpha", so we can leave the old "copyToWithAlpha" alone.

I'd be happy to implement what's necessary to get this functionality and then send post the .diff.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby CuteAlien » Tue Dec 05, 2017 11:08 pm

@chronologicaldot: So you have one blitter function which works for you? I'm still unclear on that.
If you have - which patch would that one exactly need?
And then - yeah - I guess we can make a new blitter function - if "combine" makes sense - sure. But have to check then if it's ok to implement new blitter operations only for one color format (32-bit). On first view again it looks like that might be fine. And adding a parameter to copyToWithAlpha should be enough - at that place an additional check to select another blitter shouldn't matter.

edit (yeah again...): Should probably be BLITTER_TEXTURE_ALPHA_COMBINE and/or BLITTER_TEXTURE_ALPHA_COLOR_COMBINE. As copyToWithAlpha seems to use the BLITTER_TEXTURE_ALPHA_BLEND and BLITTER_TEXTURE_ALPHA_COLOR_BLEND.
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8528
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Tue Dec 05, 2017 11:21 pm

@cutealien - The blitter function that works for me is the one whereby it ends with:
cpp Code: Select all
 
 u32 sa = c1 >> 24;
 u32 da = c2 >> 24;
 // Commented out because it's done earlier in the function:
 //alpha = sa + ( sa >> 7 ); // stretch [0;255] to [0;256] to avoid division by 255. use division 256 == shr 8
 u32 blendAlpha_fix8 = (sa*256 + da*(256-alpha))>>8;
 return blendAlpha_fix8 << 24 | rb | xg;
 

When I tested this version, I got the result I wanted. However, I only had one visual test, so I want to perform more tests to make sure it is correct. Admittedly, this version of PixelBlend32 isn't in accordance with the true corresponding OpenGL version, which is what burningreggae was saying. That's why, if we wanted to keep SoftwareDriver2/Burnings as being the same as OpenGL, then we would leave the current implementation of PixelBlend32 alone and just create a new, separate function chain for combining alpha values the way I would prefer.

If we want to put it in copyToWithAlpha, that's fine. We could call the parameter "bool combineAlpha" and set it default to "false" for backward compatibility.

If you want me to implement this alpha-combining functionality for more color types, I can try to do that, but it'll take me longer to create a patch.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby CuteAlien » Tue Dec 05, 2017 11:28 pm

I'd be glad to get a patch. And "bool combineAlpha" would also be fine by me. Taking longer is no problem, you are the person who needs that right now - so unless you run out of time you have zero pressure. Take all the time you need :-)

edit (just because edits are fun now ^_^): If you know a concrete algorithm name for this (like this kind of handling alpha's is know as the chronologicaldotAlphaCombine in literature) it's even better than using a generic name like combine. But if not - "combineAlpha" and quick comment that this parameter flips between using source-alpha and combining source and destination alpha is also OK.
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8528
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Tue Dec 05, 2017 11:57 pm

Given what it says here: https://en.wikipedia.org/wiki/Painter%27s_algorithm
I suppose we could call it "paintAlpha", "painterAlpha", or "paintersAlpha". Any one of these would be sufficient I suppose. Do you think this is descriptive enough?
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby CuteAlien » Wed Dec 06, 2017 1:22 am

Hm, don't think it fits for this. Let's stay with combineAlpha.
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8528
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Wed Dec 06, 2017 1:42 am

Ok.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Diff files

Postby chronologicaldot » Wed Dec 06, 2017 5:18 am

I added the "combine alpha" functionality and made it accessible via IImage::copyToWithAlpha(). I didn't do it for the software drivers because that would require adding the parameter bool combineAlpha to the draw2DImage function AND maybe adding a listing to the video driver features enum, and I didn't know what to call it.

Turned out to be easier than I expected to implement the "combine alpha" functionality. A number of existing functions were used when no alpha was available in the source image.
Here are the diff files:
CBlit.h - https://pastebin.com/J0ruYqY0
IImage.h - https://pastebin.com/zYBsMUg2
CImage.h - https://pastebin.com/YPdSL48d
CImage.cpp - https://pastebin.com/M32beLu2
SoftwareDriver2_helper.h - https://pastebin.com/GAmsVA6z

I didn't have any 16-bit images on hand to test the 16-bit version, so no guarantees. I hope the simplicity of their alpha channels made it less likely for me to have messed them up.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby CuteAlien » Wed Dec 06, 2017 11:15 am

Oh wow, you did 16-bit formats as well? Nice work! On which Irrlicht version is the diff based - current Irrlicht trunk? (thought probably not much has changed anyway in those files between 1.8 and trunk ...)
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8528
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby CuteAlien » Wed Dec 06, 2017 9:00 pm

Patch looks good, applied to svn trunk r5590. I hope it works out like that, otherwise you can just send more patches (before Irrlicht 1.9 release it's no problem changing it further). Thanks!
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8528
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Re: [BUG+FIX] Blitting wrong alpha values (Burnings and IIma

Postby chronologicaldot » Fri Dec 08, 2017 3:31 am

For confirmation, yes, it was the current trunk.
User avatar
chronologicaldot
Competition winner
 
Posts: 560
Joined: Mon Sep 10, 2012 8:51 am

Previous

Return to Bug reports

Who is online

Users browsing this forum: No registered users and 1 guest