gonetz / GLideN64

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

Episode 1 racer broken LOD? #2615

Closed fzurita closed 2 years ago

fzurita commented 2 years ago

It looks like recent changes broke episode 1 racer. Far away textures look black and turn the normal color as you get closer in some areas. This is visible in the very first level.

Star Wars Episode I - Racer (U)  !

The fast shader path doesn't have this problem.

fzurita commented 2 years ago

@gonetz @standard-two-simplex this broke very recently.

ghost commented 2 years ago

@gonetz I debugged the situation a little. Only the textures outside of the track use LOD. The texture has 4 tiles and it uses a standard combiner equation

(tile1 - tile0)*lod_frac + tile0

When tile0==0 the texel is fetched correctly from a normal texture. When tile0>0 or tile1>0 the texel is fetched from the atlas but it seems to return black (probably fails).

Therefore, pixels near the camera render correctly, but pixels far away are black.

I don't understand why the atlas is not working. TextureCache::_loadAccurate() seems to set up _pTexture.max_level=3 correctly and does atlas related operations.

gonetz commented 2 years ago

I'll check it asap.

gonetz commented 2 years ago

It seems that uTex1 sampler points to a wrong texture.

ghost commented 2 years ago

I was looking into it at the moment.

The texture has 4 tiles and it uses a standard combiner equation

I was wrong in this, there is one such texture but it is not the affected one.

The affected one has uMaxTile == 0 but detail is enabled. So the shader generates indices 0 and 1 and the second one is sampled from the atlas. But I believe that textures.cpp does not set up an atlas because max_level==0.

gonetz commented 2 years ago

The affected one has uMaxTile == 0 but detail is enabled. So the shader generates indices 0 and 1 and the second one is sampled from the atlas. But I believe that textures.cpp does not set up an atlas because max_level==0.

Yes, we don't need texture atlas in that case. What is RDRAM address of the affected texture? 0x801E3280 ?

Edit: I checked the code. "textures.cpp does not set up an atlas because max_level==0" - true. This part should be corrected since mipmap code expect texture atlas for the second texture.

ghost commented 2 years ago

This part should be corrected since mipmap code expect texture atlas for the second texture.

Mmm, I realize another thing. The shader has one piece of code where if uMaxTile is 1 the texel is fetched from a normal texture

"  else if (uEnableLod == 0 || uMaxTile == 1) {READ_TEX0_MIPMAP(readtex0, uTex1, tcData1);}\n"

But in the detailed case, uMaxTile == 0 because of the way the uniform is set. Reading from the normal uTex1 texture seems to work.

gonetz commented 2 years ago

" else if (uEnableLod == 0 || uMaxTile == 1) {READ_TEX0_MIPMAP(readtex0, uTex1, tcData1);}\n"

The problem is with tex1, but it has the same condition else if (uEnableLod == 0 || uMaxTile == 1) {READ_TEX0_MIPMAP(readtex1, uTex1, tcData1);}

So, the only thing is to correct uMaxTile uniform?

gonetz commented 2 years ago

Fix:

    void update(bool _force) override
    {
        uMinLod.set(gDP.primColor.m, _force);
-       const CachedTexture * _pTexture = textureCache().current[1];
-       if (_pTexture == nullptr)
-           uMaxTile.set(gSP.texture.level, _force);
-       else
-           uMaxTile.set(_pTexture->max_level > 0 ? gSP.texture.level : std::min(gSP.texture.level, 1u), _force);
+       u32 maxLevel = gDP.otherMode.textureDetail == G_TD_DETAIL ? gSP.texture.level + 1 : gSP.texture.level;
+       uMaxTile.set(maxLevel, _force);
        const int uCalcLOD = (gDP.otherMode.textureLOD == G_TL_LOD) ? 1 : 0;
        uEnableLod.set(uCalcLOD, _force);
        uTextureDetail.set(gDP.otherMode.textureDetail, _force);
    }
ghost commented 2 years ago

So, the only thing is to correct uMaxTile uniform?

gSP.texture.level determines the maximum lod, so passing its value increased by one is not a good idea. We should change the condition in the shader instead. Something like,

bool bUseNoAtlas = uEnableLod == 0 || (uTextureDetail == 2 && uMaxTile == 0) && (uTextureDetail != 2 && uMaxTile == 1);

Or perhaps send a new uniform that contains this boolean.

gonetz commented 2 years ago

It seems that a new uniform is a better solution.

gonetz commented 2 years ago

It seems that a new uniform is a better solution.

Done: https://github.com/gonetz/GLideN64/pull/2618

gonetz commented 2 years ago

Fixed