gonetz / GLideN64

A new generation, open-source graphics plugin for N64 emulators.
Other
770 stars 179 forks source link

Lack of dithering in N64 framebuffer causing color banding #2173

Closed TorutheRedFox closed 4 years ago

TorutheRedFox commented 4 years ago

Dithering is honestly necessary to produce a correct (and actually somewhat good looking) N64 framebuffer, as it's quantized with no dithering, which deviates from hardware, which does infact have dithering

oddMLan commented 4 years ago

Also, I see no need to force particular dithering mode. Currently plugin uses dithering mode which is set by the running game.

I didn't see the point either, but then I tested MK64 and noticed it actually has no dithering (fact checked this Angrylion as well). However in @gizmo98's screenshots look like he's forcing the dithering and it actually makes the skies and road look better (and the jumbotron). So there is a benefit in case there's banding and the game itself doesn't enable dithering. Or of course, if someone happens to prefer a particular dithering pattern over other. I personally prefer the Magic Square pattern.

But what's a bit weird is that it seemed some options were forcing bayer dithering always in the test ROM, but I can't seem to get dithering at all with those options in MK64 for some reason.

If you disable dithering in test rom but still see dithering pattern, it's a bug. However, make sure that you disabled dithering in RDP, not in VI. May be it would be more correct to implement shader dithering, which corresponds to RDP dithering and works per-polygons and another one, which corresponds to VI dithering and works as post-processing. It can be hard to implement properly.

I guess the ultimate test would be making the dithering pass this N64 rom test. Per-polygon like you said might be very tricky to implement at this time. I dont know exactly what this test rom is doing, so you might want to check it.

Btw the test rom doesn't work in older Project64 versions due to a paging bug. Here's a fixed version of the rom that works with older versions, or you can run PJ64's latest nightly which is also fixes the issue with the unfixed rom. nu0.zip

gizmo98 commented 4 years ago

@oddMLan MK64 has no dithering. I was working on a B/W dithering app at that time and took a MK64 screenshot to show how dithering could look like. All other screenshots are Gliden64 with shader dithering.

There is one screenshot which shows RDRAM dithering MK64 jumbotron. This was before the value of other mode color dither was evaluated.

gonetz commented 4 years ago

@gizmo98 I think, would not it be better to implement shader-based dithering as post-processing? I see only benefits:

gizmo98 commented 4 years ago

I tested dithering branch again with mupen64plus. ditheringMode = 0 -> No noise or ordered grid dithering visible ditheringMode = 1 -> Only noise dithering. No bayer dithering -087 -088 -090 -089 Other modes run as expected.

@gonetz I don't know. Alpha dithering makes no sense to implement as a post processing effect. Starfox seems to use it to fade in and out text.

starfox64-464 starfox64-465 starfox64-466

Color dithering could be implemented as a post processing effect if everything is dithered. Take a look a this screenshot. Am i wrong or are stars and OSD not dithered?!

If post processing dithering is applied it should also be checked what happens with things which are alpha noise dithered. Don't know if i end up with a bayer dithered light cone in CBFD: conker_bfd-226

oddMLan commented 4 years ago

@gizmo98 My tests were using "Render N64 Frame buffer to output". The frame buffer shouldnt be dithered if I disable dithering in the test rom.

gonetz commented 4 years ago

@gizmo98 good points. Ok, lets left it as it is now.

My tests were using "Render N64 Frame buffer to output". The frame buffer shouldnt be dithered if I disable dithering in the test rom.

Yes, it should not.

gizmo98 commented 4 years ago

Is "Render N64 Frame buffer to output" "EnableCopyColorFromRDRAM = true" for mupen?

Update: If EnableCopyColorFromRDRAM = true everything is bayer dithered (why?). But if nativeRes = 1 and ditheringMode = 4 everything looks as expected. So i would propose to disable RDRAM dithering if EnableCopyColorFromRDRAM = true.

gonetz commented 4 years ago

So i would propose to disable RDRAM dithering if EnableCopyColorFromRDRAM = true.

We need to find why it happens first

gizmo98 commented 4 years ago

@gonetz My idea. RDRAM dithering works like a post processing filter and looks wrong. Shader dithering works per polygon and looks right in native res. gdp color dithering value is wrong at the time ColorbufferToRDRAM is runnning.

gonetz commented 4 years ago

Yes, it is possible.

gizmo98 commented 4 years ago

@oddMLan EnableCopyColorFromRDRAM is really a good setting to test dithering!

@gonetz If correct dithering during Buffercopy is not possible because we cannot check which dithering pattern should be applied only shader dithering can be used. But RDRAM dithering with shader dithering only looks right if nativeRes = 1 or nativeRes = 1 and EnableCopyColorFromRDRAM = true or a nativeRes dithering pattern is applied to higher res output resolution like current noise dithering implementation (so RDRAM dihtering is only possible if higher res image is also dithered...).

gonetz commented 4 years ago

If we speak about correct dithering, yes. But I, personally, want to have dithered image in RDRAM, but don't want to see dithering pattern on my hi-res image. How to get it? I see two options:

Both options lead to heavy overheads. The feature is not worth it imo. I think that wrong dithering is better than no dithering. Users seldom see low-res RDRAM image and hardly will notice that the dithering pattern is wrong. If someone wants to get things right - native res is the best option to get rid of 2D issues and get authentic look, including correct dithering.

If you agree with me, I think the feature is ready to be merged.

gizmo98 commented 4 years ago

I agree with you! If someone wants correct dithering enable nativeResFactor = 1 and ditheringMode = 4 with or without EnableCopyColorFromRDRAM = true.

oddMLan commented 4 years ago

I just think the config should be reorganized to reflect the limitations, maybe in the form of a tooltip. We could go with Gonetz' idea, in which noise is permanently enabled and quantization is moved to a checkbox. As for the options in the combobox, could be as following:

Dithering:

Tooltip: Controls the behavior of dithering pattern, which adds authentic look to final picture. Only RDRAM (Default) Dithering only happens when image is copied to RDRAM which reduces color depth. This gets rid of banding artifacts. Dithering is only applied if the game itself enables it. Output and RDRAM Same as above, except that dithering pattern is applied to final on-screen picture as well. This option is the most accurate when playing in native resolution.

Enable quantization The N64 adds 5-bit quantization pass to the image after dithering, however this makes the dithering pattern and noise look less homogeneous and some people may dislike it, so it's disabled by default. For a more authentic look you can enable it.

EDIT: I would like the 2x noise option replace the defunct "Enable Noise" under the Emulation video tab... it looks fantastic :)

gizmo98 commented 4 years ago

@gonetz If gDP.otherMode.colorDither cannot be checked during buffercopy switch statement can be removed: https://github.com/gonetz/GLideN64/blob/dithering/src/BufferCopy/ColorBufferToRDRAM.cpp#L267 The best looking dithering method should be selected for RDRAM dithering.

We could also think about always dithering RDRAM content with an even better dithering method which does not produce noticable dithering fragments (floyd steinberg or blue noise dithering). Blue noise dithering can be with shader dithering and a blue noise texture. Just dither image with this noise texture (like the current noise dithering implementation) after it is resized to RDRAM resolution. http://momentsingraphics.de/BlueNoise.html https://forums.tigsource.com/index.php?topic=40832.180

gonetz commented 4 years ago

If gDP.otherMode.colorDither cannot be checked during buffercopy

We rather can't rely on it.

The best looking dithering method should be selected for RDRAM dithering.

Which method is the best looking? Or make another option for it?

We could also think about always dithering RDRAM content with an even better dithering method which does not produce noticable dithering fragments

I don't mind. Can you implement it? If yes, user option for dithering method will not be necessary.

gizmo98 commented 4 years ago

I don't know how to load a texture but i know how to open a bitmap and write it to a c array.

static const s32 blueNoise[640][580] = { 
 // Add content
};

u16 ColorBufferToRDRAM::_RGBAtoRGBA16(u32 _c, u32 x, u32 y) {
    union RGBA c;
    c.raw = _c;

    s32 thresholdR = blueNoise[x][y];
    s32 thresholdG = blueNoise[639 - x][y];
    s32 thresholdB = blueNoise[x][579 - y];     

    c.r = (u8)std::max(std::min((s32)c.r + thresholdR, 255), 0);
    c.g = (u8)std::max(std::min((s32)c.g + thresholdG, 255), 0);
    c.b = (u8)std::max(std::min((s32)c.b + thresholdB, 255), 0);

    return ((c.r >> 3) << 11) | ((c.g >> 3) << 6) | ((c.b >> 3) << 1) | (c.a == 0 ? 0 : 1); 
}

This should be good enough to test if blue noise dithering looks better.

gonetz commented 4 years ago

@gizmo98 I implemented blue-noise dithering in 37a59ee741

Note: I added temporal setting BufferDitheringMode with options (0=disable, 1=bayer, 2=magic square, 3=blue noise) I implemented it only for mupen64plus, I don't want to add it to GUI. This setting can be used to test various modes and select the best one.

How it works. I downloaded blue-noise png textures from http://momentsingraphics.de/BlueNoise.html took eight 64x64 ones from LDR_RGB1_1 set and converted them into C array blueNoiseTex. Values in png texture have type unsigned 8bit integer, but, as I understand, actual type is signed 8bit. Threshold applied as c.r = (u8)std::max(std::min(((((s32)c.r) << 5) + threshold.r) >> 5, 255), 0); Original u8 color converted to s32 and shifted left by 5. It is done to apply threshold only to 3 LSB of the color. After threshold is added, result shifted right by 5 and clamped.

Please check that commit carefully, I could make an error.

Result: DitheringCompare

oddMLan commented 4 years ago

It certainly looks more uniform and natural. Although the other dithering modes are more accurate. Should there be an option for accuracy? Even if you can't rely in gDP.otherMode.colorDither 100%, shader dithering worked properly in the test rom with ditheringMode = 4. I like that this plugin allows choosing if you want accuracy or eye candy. Enhancements like point lighting and hi res are optional, so blue noise should be too imo.

Revising my previous suggestion, it could be something like this:

Dithering (whatever the game chooses):

☑ Enable quantization ☑ Enhanced dithering (for blue noise dithering) ☑ High resolution noise (for 2x dithering noise)

Tooltips: Enhanced dithering: Uses a different method for dithering which avoids repetitive patterns of tradional N64 dithering.

Cons: you don't really see dithering pattern in final picture in a real N64, the VI filters get rid of it, so the method chosen might not matter after all.

gizmo98 commented 4 years ago

@gonetz i tested blue noise dithering yesterday with this branch. https://github.com/gonetz/GLideN64/compare/master...gizmo98:patch-1?expand=1

I converted a 256x256 blue noise texture to a s8 array. I precalculated the threshold value: threshold = (color >> 5) - 4; So the threshold value is in the range of 4 to -4 and moves around 0.

I will review your code tomorrow.

I cannot see dithering artifacts with blue noise dithering: mariokart64-056 conker_bfd-228 mariogolf64-033

@oddMLan which Mario Tennis picture looks better? (ditheringMode 4 vs 1) mariotennis-008 mariotennis-010 mariotennis-009 mariotennis-011

oddMLan commented 4 years ago

The second one, but nothing wrong with it if the user chooses to see the dithering pattern in the final picture. Hence I ask to give the user these options

Dithering (whatever the game chooses):

  • Only in RDRAM
  • Output and RDRAM

The first option woud be what currently is ditheringMode = 1, the second would be ditheringMode = 4

Also, that Conker pause screen with blue noise dithering looks great.

gonetz commented 4 years ago

@oddMLan I don't think that blue noise dithering is useful as shader dithering. Output is 32bit, dithering is not needed. We have shader dithering for authentic look only, so it must be N64 dithering. We have 3 situations:

As you see, shader dithering and post-processing dithering must be controlled independently. The idea was to choose best-looking dithering method for post processing and always use it, without making a user setting for it. But it can be controlled by separate setting, as it is done in my latest commit for blue noise dithering.

I want to keep number of options as less as possible, but it is hard in this case. My proposition for settings:

  1. Noise dithering is always enabled
  2. Checkbox "Dithering pattern on output image" - enables shader dithering
  3. Checkbox "High resolution noise dithering" - enables 2x dithering noise
  4. Checkbox "Enable quantization" - for all dithering methods including noise
  5. Combo-box "RDRAM image dithering" with options:
    
    disable
    Bayer
    Magic square
    Blue noise
oddMLan commented 4 years ago

Would the RDRAM image dithering control the shader dithering too? Or which method will be used if the user chooses to enable shader dithering? As it works currently, I assume. Which is let the game decides (either Bayer or Magic Square, or none), correct? If that's the case, I believe the proposed naming is accurate and a lot less confusing than the current one! Just make sure to clarify in a tooltip that "Display dithering pattern in output" won't work with blue noise dithering (because it uses what the game says)

gonetz commented 4 years ago

Would the RDRAM image dithering control the shader dithering too?

No. As I explained, RDRAM image dithering and shader dithering are mostly independent of each other and need independent controls. Shader dithering is per-polygon and controlled by game. RDRAM image dithering is post-processing, dithering method controlled by user or hardcoded. The only case when shader dithering overrides RDRAM image dithering is "native res + shader dithering enabled".

Just make sure to clarify in a tooltip that "Display dithering pattern in output" won't work with blue noise dithering (because it uses what the game says)

Noise dithering has no pattern :)

gonetz commented 4 years ago

I converted a 256x256 blue noise texture to a s8 array.

The article strongly suggest to change noise texture each frame. Also it shows that in a tiled 64² blue noise texture repetitions are hard to spot. So I use 64x64 textures, but with 3 channels of noise for RGB. The author suggests to use 64 textures and randomly switch between them each frame, but in my case each texture takes 12kb, so even 8 textures waste 96kb of memory.

I precalculated the threshold value: threshold = (color >> 5) - 4; So the threshold value is in the range of 4 to -4 and moves around 0.

This is the right step, I did not guess to do it.

gonetz commented 4 years ago

I implemented my propositions in "dithering" branch: https://github.com/gonetz/GLideN64/tree/dithering

Since it is WIP and everything can change, I implemented new settings only for mupen64plus. GUI will be changed later.

gizmo98 commented 4 years ago

@gonetz What is enableHiresNoiseDithering doing? I have not found the location where it is used.

Blue noise dithering with changing textures per frame looks cool.

gonetz commented 4 years ago

What is enableHiresNoiseDithering doing? I have not found the location where it is used.

I made changes for enableHiresNoiseDithering, but forgot to commit them. Done in e74d1ceb11

gizmo98 commented 4 years ago

I added quantization because there were white remnants in SF Lylat Wars with ordered grid dithering. Noise dithering in SF Lylat Wars also looks different with quantization:

Without quantization: starfox64-486 starfox64-487

With quantization: starfox64-491 starfox64-492

How does it look with angrylion or original hardware?

Update. CopyToRDRAM and NoiseDithering without quantization looks like this: starfox64-506

gonetz commented 4 years ago

angrylion lle with vi filters off: StarFox_al

I can't get that noise with GLideN64 now. Broken?

gizmo98 commented 4 years ago

@gonetz It should be visible if quantization is disabled.

gonetz commented 4 years ago

I can't see it with and without quantization. Tested with current dithering branch and with build made from 02728994fbe "Remove enableDithering setting and use ditheringMode for everything"

gizmo98 commented 4 years ago

I will take s look later. But a real n64 does not seem to have noise 1:58: https://m.youtube.com/watch?v=BXU1YH7AkCI

oddMLan commented 4 years ago

I will take s look later. But a real n64 does not seem to have noise 1:58:

Yes, because the VI filters have a dedither filter that removes the dithering noise. The screenshot Gonetz showed has VI filters disabled, which should match our case here (since aside gamma correction, those are not emulated in GLideN64)

Maybe the gamma is being applied in the wrong order? Eg. quantized, then gamma corrected. When it should be gamma corrected, then quantized. Somebody take a look at angrylion to see what it's doing.

gonetz commented 4 years ago

The screenshot Gonetz showed has VI filters disabled, which should match our case here

Yes, it should.

Maybe the gamma is being applied in the wrong order?

Quantization goes first of course since gamma correction is post-processing. But I don't think that it is related.

gonetz commented 4 years ago

It seems that angrylion-plus, which I used to take the screen shot, does not disable vi filters completely. I built original angrylion plugin from sources - noise function called only from vi filters in that place of Star Fox. I disabled vi filters and got this: al_original_no_vi_filters

gizmo98 commented 4 years ago

@gonetz There are vi filters which add noise? VI filters with noise and noise dithering are different things? Dithering is applied before image is written to RDRAM and VI filter filters RDRAM image?

So there is one question left. Why does your picture look the same with and without quantization. I'm quite sure i tested latest sources with mupen64plus.

Update: Build from source. Still noise without quantization. And with quantization there is still a noise effect in Zelda MM and CBFD.

Settings:mupen64plus.txt starfox64-507 starfox64-508

gonetz commented 4 years ago

There are vi filters which add noise? VI filters with noise and noise dithering are different things? Dithering is applied before image is written to RDRAM and VI filter filters RDRAM image?

UpdateScreen API function calls rdp_update(), which prepares image to display. It calls gamma_filters function. gamma_filters function uses a bit of randomness for dithering, see (line 10528): https://sourceforge.net/p/angrylions-stuff/code/112/tree//trunk/mylittle-nocomment/n64video.cpp

So there is one question left. Why does your picture look the same with and without quantization.

I use native res. The difference is clearly visible: quantization

Update: Build from source. Still noise without quantization. And with quantization there is still a noise effect in Zelda MM and CBFD.

I don't quite understand what do you mean here, but I see that there is something wrong with settings. Noise in Star Fox appears and disappears with different combination of settings.

gizmo98 commented 4 years ago

@gonetz I thought quantization is not working at all for you and i wasn‘t sure if my test setup was up to date.

gonetz commented 4 years ago

Update for noise in Star Fox: quantization kills the noise. Noise appears only without quantization I mean "press start" screen, when you press start and the screen shrinks.

gizmo98 commented 4 years ago

@gonetz But there is also no noise in your angrylion screenshot. So is it right or wrong?

gonetz commented 4 years ago

I suppose, this is right. I debugged angrylion again. It uses noise in "press start" screen only in vi filters, but when I press "start" it uses noise for fillrects, which perform screen shrinking effect. Nevertheless, noise is not visible without vi filters. So, it seems that quantization works correct here.

gonetz commented 4 years ago

I mean, thanks to quantization our picture looks correct. Noise should not be visible.

gonetz commented 4 years ago

@gizmo98 As I see, that dithering works correct and all options work as expected. Noise quantization works even in hi-res, since noise on "press start" screen disappears with it in hi-res too. Do you see any unsolved problems?

gizmo98 commented 4 years ago

@gonetz I see no unsolved problems. One last thing. We should think about enabling quantization for noise dithering by default if it looks right.

tony971 commented 4 years ago

Is there any reason noise dithering with quantization shouldn't always be enabled?

gizmo98 commented 4 years ago

@tony971 Quantization converts RGB to RGB555 and 8Bit alpha to 5Bit alpha if dithering is used. So in theory it should look worse, because you loose precision. On the other hand it looks right, because real hardware is doing the same thing.

tony971 commented 4 years ago

@gizmo98 so it's more accurately portraying N64 graphics and solving the original issue at the beginning of the thread. Sounds like a good thing to enable and only have as an ini option to disable (for people who want to get weird with upscaling and such).

gonetz commented 4 years ago

I implemented dithering settings in GUI, 827b69816f5. Quantization is on by default. Please test.

@gizmo98 I made everything but tooltips. Could you write them? configDialog.ui should be modified for that. Find Tooltip to be here text , replace it with something useful and give me a patch.

gonetz commented 4 years ago

Dithering feature is in the master. Thanks to everyone who helped us to develop and test it.