coelckers / prboom-plus

This is a cleaned up copy of the PrBoom+ SVN repository as a courtesy for those interested in forking that port
294 stars 118 forks source link

Color Translation/Fullbright Inconsistencies #545

Open T-Will133 opened 2 years ago

T-Will133 commented 2 years ago

Playing through Knee-Deep in Knee-Deep in ZDoom has revealed some inconsistencies in PrBoom+/DSDA color translation and fullbrights compared to other ports including Chocolate Doom. Two examples in PrBoom+um 2.6.2 are the imp's teeth and the Nightmare Demon.

doom00 doom01

The automap colors are also translated wrong. doom02

Oddly enough though the recent builds of PrBoom+um have fixed the Nightmare Demon. doom01

XaserAcheron commented 2 years ago

A couple of anecdotal details from various Discord convos that may help sniff out the specifics:

fabiangreffrath commented 2 years ago
  • The automap bug is caused by an old change in Boom's source -- the automap background color got swapped from 0 to 247. This is technically fixable user-side since the colors are customizable, but it'd be good to have the defaults fixed up.

https://github.com/coelckers/prboom-plus/pull/546

fabiangreffrath commented 2 years ago
  • For the imp teeth, the wad's palette has a set of fullbright colors, including a second pure-white shade. There's presumably some bit of code somewhere that's "optimizing" it by treating it as a duplicate color, and the non-fullbright white in the imp sprites are getting replaced with the fullbright one.

That probably here: https://github.com/coelckers/prboom-plus/blob/bf872a00fc4b368c3b4f0cea13321829e40fd1e8/prboom2/src/r_patch.c#L127-L159

esselfortium commented 1 year ago

I believe GZDoom's recent fix for this bug was to also check the colormap to see if both seemingly-identical colors are also treated the same in that, so that fullbrights are still differentiated as they should be.

kraflab commented 1 year ago

Duplicate detection: https://github.com/kraflab/dsda-doom/commit/708db98c8ea35ecdda124508a922f1d5ba2e0143 I added mapcolor_* OPTIONS support for dsda-doom but that's probably out of scope for pr+.