MiSTer-devel / NES_MiSTer

GNU General Public License v3.0
172 stars 75 forks source link

Incorrect color emphasis behavior when two bits are set #263

Closed BMF54123 closed 2 years ago

BMF54123 commented 3 years ago

In the MiSTer NES core, enabling two color emphasis bits simultaneously causes all three channels to be darkened. As a result, you cannot get yellow, magenta, or cyan tints.

MiSTer core: image

AV Famicom (stock composite output): image

This behavior occurs regardless of which palette is selected in the menu.

birdybro commented 3 years ago

Do you have a link to this test rom?

BMF54123 commented 3 years ago

full_palette.zip

birdybro commented 3 years ago

This should be fixed in the latest unstable build.

https://github.com/MiSTer-unstable-nightlies/NES_MiSTer/releases/download/unstable-builds/NES_unstable_20210906_f36f.rbf

BMF54123 commented 3 years ago

Well, there's some color now, but it is really faint compared with the real Famicom capture above. Tested both the unstable build and the latest build from 9/10/21:

image

A side-by-side comparison of range $x0 makes it much more noticeable (MiSTer on left, Famicom on right):

image

sorgelig commented 3 years ago

this is my version: NES.zip

it has slightly different tints on some enphasis, but at least it gives different tints in all groups. Currently core operates in rgb pixel level for tint while it should: 1) use original phase coded color. 2) use YIQ color space.

Each emphasis bit uses 1/3 of whole phase range and thus it doesn't affect cleanly R, G or B. So enphasis has "dirty" tint. That's why real NES has dirty green in "011" tint mode while my version (using RGB color space on pixels level) produces logically Yellow tint. Probably, instead of making tint over-complicated, we can hardcode tint substraction table. it won't be 100% correct but will be hard to distinguish.

p.s.: correct term would be de-emphasis. So each bit is de-emphasing by some tint (by 1/3 of phase range) but after phase and color space conversions it turned to RGB emphasis which is technically wrong point of view.

sorgelig commented 3 years ago

this build has some adjustments: NES_adj.zip

BMF54123 commented 3 years ago

That is definitely a huge improvement! However, it appears that the entire image is now shifted right by one pixel (e.g. the right-most pixel on the screen is missing, and there is an extra black pixel at the left edge). Cropping and resolution settings don't seem to affect it.

sorgelig commented 3 years ago

That's easy to correct. I will proceed with new emphasis.

Kitrinx commented 2 years ago

is this resolved? There's only a handful of games that use emphasis at all, mostly for just dimming the screen.

birdybro commented 2 years ago

https://github.com/MiSTer-devel/NES_MiSTer/commit/abddbc94824d46cdde0d7dbea90bd39046d03b14 - Sorgelig committed this last year, so it should be fixed. Testing to be sure:

Screenshot 2022-04-04 07-09-47 - Kitrinx Palette on makes it a little too subtle on the right to show that it's working.

Screenshot 2022-04-04 07-12-27 - Kitrinx High Saturation Palette shows it.

A little better. Not sure how to replicate @BMF54123's results completely since it seems to be palette dependent, but the emphasis bits are showing.

This is an interesting result: Screenshot 2022-04-04 07-14-33 - the PC-10 Better Palette shows all black for the right column. However if we go to the original PC-10 Palette: Screenshot 2022-04-04 07-21-53 - The right column does show up. I think this indicates that the PC-10 Better Palette should probably be adjusted somewhat?

Kitrinx commented 2 years ago

this is not a bug image

333,014,006,326,403,503,510,420,320,120,031,040,022,000,000,000
555,036,027,407,507,704,700,630,430,140,040,053,044,000,000,000
777,357,447,637,707,737,740,750,660,360,070,276,077,000,000,000
777,567,657,757,747,755,764,772,773,572,473,276,467,000,000,000

You'll notice the 0x0D column does not have the two lowest values. See here for more information: https://www.nesdev.org/wiki/PPU_palettes