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

Some blending doesn't look good #2053

Open
2 of 7 tasks
MarcelHB opened this issue Mar 6, 2024 · 13 comments
Open
2 of 7 tasks

Some blending doesn't look good #2053

MarcelHB opened this issue Mar 6, 2024 · 13 comments
Labels

Comments

@MarcelHB
Copy link
Collaborator

MarcelHB commented Mar 6, 2024

Bug description

While fixing and looking at #2050 more closely, I found that our blending doesn't look as good as in the originals, maybe not the same blend mode? For the character animations it's the AA_BLEND flag at play, the spell is probably analogue.

Steps to reproduce

  1. Cast spells, fire, acid effects ...

Expected behavior

See below.

Screenshots

GemRB
blending_gemrb

Native
blending_native

GemRB version (check as many as you know apply)

  • master as of this issue
  • 0.9.2
  • 0.9.1
  • 0.9.0

Video Driver (check as many as you know apply)

  • SDL1.2
  • SDL2 built with OPENGL_BACKEND enabled
  • SDL2 without OPENGL_BACKEND enabled
@MarcelHB MarcelHB added the bug label Mar 6, 2024
@bradallred
Copy link
Member

bradallred commented Mar 6, 2024

I still question what we are doing with the palettes. Can you add the BAMs in your screenshots to the issue so it's easier to investigate?

The reason my first suspicion is palette related, is because prior to ea4c25f we used to get a similar bad look for projectiles. See 21c16d0 for additional context.

@bradallred
Copy link
Member

bradallred commented Mar 6, 2024

If it's not a palette issue, then my next guess would be using a different blend mode as you suggest. Projectiles use a two bits to determine which mode to use so we may need to follow that same logic:

#define PTF_TRANS 128 // glBlendFunc(GL_ONE_MINUS_DST_COLOR, GL_ONE);
#define PTF_BRIGHTEN 256 //brighten alpha; CPROJECTILEBAMFILEFORMAT_FLAGS_BRIGHTEST in bg2
#define PTF_BLEND 512 // glBlendFunc(GL_DST_COLOR, GL_ONE);
#define PTF_TRANS_BLEND (PTF_TRANS | PTF_BLEND) // glBlendFunc(GL_SRC_COLOR, GL_ONE); IWD only?

the corresponding blend modes from the comments should be one we have defined here:

BLENDED = 8, // dstRGB = (srcRGB * srcA) + (dstRGB * (1-srcA)), dstA = srcA + (dstA * (1-srcA))
ADD = 0x10, // dstRGB = (srcRGB * srcA) + dstRGB
MOD = 0x20, // dstRGB = srcRGB * dstRGB
MUL = 0x40, // dstRGBA = (srcRGBA * dstRGBA) + (dstRGBA * (1 - srcA))
ONE_MINUS_DST = 0x80, // dstRGBA = (srcRGBA * (1 - dstRGBA)) + (dstRGBA)
DST = 0x100, // dstRGBA = (srcRGBA * dstRGBA) + (dstRGBA)
SRC = 0x200, // dstRGBA = (srcRGBA * srcRGBA) + (dstRGBA)

I'm not sure that I actually implemented them all on the backend tho.

@MarcelHB
Copy link
Collaborator Author

MarcelHB commented Mar 6, 2024

It's (IWD2) ACIDH.BAM and CONJUCG.BAM as shown in NI:
acid_bams

@lynxlynxlynx
Copy link
Member

I doubt a bit these two are from projectiles — one is clearly a casting glow (so VVC/BAM in fx_casting_glow).

@bradallred
Copy link
Member

I know they aren't projectiles, I'm speculating that they ought to behave in a similar fashion in regards to blending. I'm also pointing out that we used to do things incorrectly with projectiles with their palettes/blending and perhaps we were doing the same type of thing here.

In general, I'm skeptical of the type of palette manipulation I removed in 21c16d0 and I doubt it was the only place we were doing it.

It's (IWD2) ACIDH.BAM and CONURCG.BAM as shown in NI: acid_bams

since those have a white instead of black background, ONE_MINUS_DST probably isn't the correct mode, but maybe one of the others.

@lynxlynxlynx
Copy link
Member

They're transparent (green), I guess the white is from screenshoting (and the second one is actually CONJUCG.BAM).
For the splash translucency might be enough, but not sure about the casting glow, the brightening could be a visual trick, since the bg is darker.

@bradallred
Copy link
Member

No, these need special blending. It's the only way to achieve a gradient blend in the original engine and it's clear from the screenshot that there is no gradient. It should be easy to test. If it's a VVC, just force IE_VVC_BLENDED to see if that does it. Then if not, go tweak the blending used for the IE_VVC_BLENDED case to try the others. It's also possible we need a new blend mode, but I doubt it.

we do have this hack that might need to be tweaked/resolved, but maybe that hack is actually what makes this work with BAMs with a regular color key such as these:

if (flags & (BlitFlags::ONE_MINUS_DST | BlitFlags::DST | BlitFlags::SRC)) {
// FIXME: this is a hack. the video driver just needs to be able to ignore the color key during any blending
if (pal && pal->col[0] != ColorBlack) {
ck = pal->col[0];
pal->CopyColorRange(&ColorBlack, &ColorBlack + 1, 0);
}
}

@bradallred
Copy link
Member

Maybe it's not relevant, since IWD2 doesn't seem to use VVCs, but I do see that the VVC format has an "alpha" field that we ignore. Not sure if it just isn't used by the originals or what. It's not set on any of BG2s VVCs. Maybe on some of the hardcoded ones tho? 🤷🏻‍♂️

@lynxlynxlynx
Copy link
Member

That resref is unset in all games I have, so I'd sooner rule the mask as unused. Why reserve space for one if you don't actually provide any files for it? And why would you need it if it was constructed at runtime.

@bradallred
Copy link
Member

If it were used by anything hardcoded there would still likely be a BAM with the alpha information. Finding it might be a pain since it would be invisible in NI, but I would have suspected a similar name to the BAM its an alpha for.

I don't disagree that it's likely unused, features get axed or have alternatives developed without cleaning up the garbage all the time. But it also wasn't uncommon in that era to embed data files within binaries for speed. Don't we already deal with instances of that?

For now it doesn't matter. The obvious first thing to try is the blending.

I see that AA_BLEND does set IE_VVC_BLENDED, so then id try changing the blending used. IIRC, ONE_MINUS_DST was picked because that was what was being used elsewhere. Look at 1ab8bea for context. What is the shadowalpha for? Could that be a problem?

@bradallred
Copy link
Member

Let's also figure out which RGBModifiers are being applied to the palette.

@lynxlynxlynx
Copy link
Member

It's for actual shadows, a special case like the transparency index before it. Off the top of my head it's important for avatar animations and inventory items.

@lynxlynxlynx
Copy link
Member

SRC and DST look identical and get very close for casting glows:
image

Native from first post:
blending_native

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

No branches or pull requests

3 participants