gonetz / GLideN64

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

Fix texrects in native resolution for fast shader path #2632

Closed fzurita closed 2 years ago

fzurita commented 2 years ago

My understanding here is limited, so this solution is definitely up for discussion. See here:

https://github.com/gonetz/GLideN64/issues/2506

The real fix in this is always setting the vertexOffset to zero in the fast path.

The other change is something I noticed while in this file that is in the accurate path but appears to be missing from the fast path.

fzurita commented 2 years ago

@standard-two-simplex Could you please look at this?

fzurita commented 2 years ago

@standard-two-simplex to answer your question here:

https://github.com/gonetz/GLideN64/issues/2506#issuecomment-1008160608

Zelda in-game menus look correct with the vertexOffset set to zero in the fast path.

With vertexOffset set to 0.5, zelda HUD and in-game menus look blurry.

Ogre battle 64 dialogs look wrong with vertexOffset set to zero, it adds some gaps that don't belong. With the offset set to 0.5, they look correct.

fzurita commented 2 years ago

Ok, this produces the correct result for both situations:

const float vertexOffset = isNativeRes && gDP.otherMode.textureFilter == 0 ? 0.5f : 0.0f;

And Mario Kart 64 still looks correct.

ghost commented 2 years ago

Maybe you can try to adjust the texture filter. I think it should go,

"  mediump vec2 offset = fract(texCoord*texSize);           \n"

to compute the correct fractional part.

And

"#define TEX_OFFSET(off) texture2D(tex, texCoord - (off + 0.5)/texSize)         \n"

to make sure that texture2D() is called on a normalized half integer texture coordinate.

I believe this changes will work with the vertex offset.

fzurita commented 2 years ago

I'm a little confused, are you saying that I should do that in addition to

const float vertexOffset = isNativeRes && gDP.otherMode.textureFilter == 0 ? 0.5f : 0.0f;

or that the above is wrong and I should do what you suggest in the shader instead while leaving the vertex offset as zero?

ghost commented 2 years ago

You should do the above changes to the texture filter and a vertex offset of 0.5 always (in native) for the correct behaviour.

fzurita commented 2 years ago

Ok, I did that. Legend of Zelda and Ogre battle 64 still look ok. Unfortunately, Mario Kart 64 got broken by that.

Did I do it as you expected?

https://github.com/mupen64plus-ae/mupen64plus-ae/commit/7743923c64d1c08667b00136b8e2367f2673de34

ghost commented 2 years ago

I realize it should be

"#define TEX_OFFSET(off) texture2D(tex, texCoord - (off + vec2(0.5))/texSize)           \n"

but unless you have shader compilation errors, it shouldn't matter.

Zelda is fixed but Mario Kart isn't? You're testing in native I guess?

Edit: I see the error, I intended to add 0.5 but there is a minus sign before. So it should be something like this.

"#define TEX_OFFSET(off) texture2D(tex, texCoord - (off - vec2(0.5))/texSize)           \n"
fzurita commented 2 years ago

I am testing in native, I'll try that.

fzurita commented 2 years ago

Everything looks correct after that change. I'll update the pull request.

gonetz commented 2 years ago

Everything looks correct after that change.

Unfortunately, it does not. Mario Kart is not completely fixed in native, copyright looks wrong: MK_(C)

What is much worse, non-native resolutions now look completely wrong even with native-res texrects enabled. It was correct before the fix.

fzurita commented 2 years ago

Ok, I've been looking at a low resolution screen, so that is hard to tell for me.

How does it look with my original change of just this?

const float vertexOffset = isNativeRes && gDP.otherMode.textureFilter == 0 ? 0.5f : 0.0f;
gonetz commented 2 years ago

MK64 title screen, fast path

It seems that the vertexOffset is not applicable for the fast path.

fzurita commented 2 years ago

Ogre battle 64 dialogs look wrong with vertexOffset set to zero, it adds some gaps that don't belong. With the offset set to 0.5, they look correct.

Maybe just ignore that case and live with the fast path being less accurate?

gonetz commented 2 years ago

Yes, OB64 is a special case. It works correct with accurate path, so I think we can ignore its issues with fast path.

ghost commented 2 years ago

It seems that the vertexOffset is not applicable for the fast path.

I have been checking the situation a bit. I had the same issue with the accurate path.

The game generates integer texture coordinates. For textures using nearest neighbour filtering mode, if the texture coordinates were slightly smaller, an incorrect texel would be fetched. While approximating this behaviour with OpenGL, some precision errors are made. In order to avoid this, I introduced a small bias in ShaderFragmentCorrectTexCoords().

            " highp vec2 mTexCoord0 = vTexCoord0 + vec2(0.0001);                        \n"
            " highp vec2 mTexCoord1 = vTexCoord1 + vec2(0.0001);                        \n"

This prevents the previous precision issue, while it is very unlikely that a texture coordinate intended to have fractional part 0.9999 overflows to the next integer because of this.

However, in the fast path all the code in ShaderFragmentCorrectTexCoord() is being completely ignored. Which means neither the small bias, nor the texture offsets passed via uTextureOffset() are being used at all. On top of this, I realize that the values computed for uTextureOffset() are not being normalized, so those offsets should be divided by texture size. Otherwise, they could be computed exactly as in the accurate path.

From my understanding, the inaccurate path doesn't emulate clamp wrap and mirror correctly on texture edges (remember the floor on the graveyard in MM) or the per pixel shift and offset. This should be edge cases, so I believe that most of the time it is possible to achieve an accurate image.

gonetz commented 2 years ago

From my understanding, the inaccurate path doesn't emulate clamp wrap and mirror correctly on texture edges (remember the floor on the graveyard in MM) or the per pixel shift and offset.

Yes. Fast path does less per-pixel calculations, that's why it is fast :)

This should be edge cases, so I believe that most of the time it is possible to achieve an accurate image.

Could you make a proper fix then?

ghost commented 2 years ago

Could you make a proper fix then?

I will try. However, it might be a bit complicated to get it all right. The accurate path had lots of regressions that I had to debug and fix.

fzurita commented 2 years ago

It may not be worth it, the accurate path is the path forward, so any fixes in the fast path have a limited life span. We could just leave the fast path with the bugs and accept that it's not as good.

ghost commented 2 years ago

Okay, I will try to do a quick fix, see if most things work. If they don't, it can be discarded. In the meantime, you may merge this PR if you want MK to work.