some pending renderer regressions introduced in alpha 38 and alpha 40 #799

illwieckz · 2015-08-22T01:38:25Z

Hi, these regressions are not spotted by current unvanquished assets, but this foreign assets show things that worked before and are now broken, so unvanquished things can break in the future.

How looks the railgun map from Wolf:ET on unvanquished 0.37.0:

Only the snow particle is missing (perhaps it never worked).
How looks the railgun map from Wolf:ET on unvanquished 0.38.0:

Many surface shaders are broken, trees are broken.
How looks the railgun map from Wolf:ET on unvanquished 0.40.0:

The fog is broken and the skybox too (the white background is the void).

illwieckz · 2015-08-22T02:19:00Z

For information, daemon complains about that:

WARNING: unknown shader stage parameter 'detail' in shader 'textures/snow_sd/icelake2_opaque' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_6' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_5' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_4' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_3to4' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_3' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_2to5' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_2to4' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_2to3' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_2' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_1to5' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_1to3' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_1' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0to6' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0to4' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0to3' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0to2' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0to1' 
WARNING: unknown shader stage parameter 'detail' in shader 'textures/mp_railgun/lmterrain_0'

But it was already true in 0.37.0 and it never prevented the map to be rendered.

illwieckz · 2015-08-22T02:46:23Z

How you can test it:

Download official etlegacy assets from an official mirror (you can use pimi):

pimi.sh etmain

Add some compatibility layer (no legal issue, it's just doing symlinks, downloading empty pk3s containing DEPS file only and downloading a spawn layout so the game does not end at startup):

mkdir -p ~/.unvanquished/pkg/etmain
mkdir -p ~/.unvanquished/game/layouts/railgun
ln -s ~/.etlegacy/etmain/pak0.pk3 ~/.unvanquished/pkg/etmain/pak0_0.pk3
ln -s ~/.etlegacy/etmain/pak1.pk3 ~/.unvanquished/pkg/etmain/pak1_0.pk3
ln -s ~/.etlegacy/etmain/pak2.pk3 ~/.unvanquished/pkg/etmain/pak2_0.pk3
wget -O ~/.unvanquished/pkg/etmain/res-etmain_0.pk3 http://dl.illwieckz.net/b/unvanquished/extra/foreign-maps/etmain/pkg/etmain/res-etmain_0.pk3
wget -O ~/.unvanquished/pkg/etmain/map-railgun_0.pk3 http://dl.illwieckz.net/b/unvanquished/extra/foreign-maps/etmain/pkg/etmain/map-railgun_0.pk3
wget -O ~/.unvanquished/game/layouts/railgun/spawn.dat http://dl.illwieckz.net/b/unvanquished/extra/foreign-maps/etmain/game/layouts/railgun/spawn.dat

Load the railgun map with the spawn layout:

daemon -pakpath ~/.unvanquished/pkg/etmain/ +devmap railgun spawn

illwieckz · 2015-08-22T23:34:03Z

Lol, between 0.37 and 0.38 there is a commit 866a9e5 that said “Remove a bunch of unused renderer stuff” and “Remove the ET fog”, but the fun thing is the fog is still there after this commit and still there in 0.38.0, and nothing else is missing, so the fog in this map does not use the ET fog.

So, yes, that were plenty of unused code. 😛

illwieckz · 2015-08-23T00:37:20Z

So, the first issue (the textures disappearing) seems to have been introduced in 3792f29 “Remove unused warnings”. But the deleted code not only remove warnings in case of error, it also delete the fallbacks.

When I blindly revert this commit, that fixes the ground textures (the trees are still broken), it's exactly this change:

@@ -1850,12 +1819,6 @@ static qboolean ParseStage( shaderStage_t *stage, char **text )
                        stage->forceHighQuality = qtrue;
                        stage->overrideNoPicMip = qtrue;
                }
-               // detail
-               else if ( !Q_stricmp( token, "detail" ) )
-               {
-                       ri.Printf( PRINT_WARNING, "WARNING: detail keyword not supported in shader '%s'\n", shader.name );
-                       continue;
-               }

The main thing is the continue call, so the whole shader does not fail. There is still a "WARNING: detail keyword not supported in shader” message in more recent releases, it probably falls on a generic warning that does not do the continue call so now the whole shader fails.

But the worst thing is the other issues, like the tree issue (introduced in same release), or the fog and skyboxes issue introduced in 0.40, there is no fallback and no error message.

It reminds my issue #776 (missing texture but no warning message), in the case of the fog, skybox and tree issue spotted in this thread, a mapper get the worst thing: a completely silent failure.

Having a shader failure with an error message is ok, having a shader fallback with a warning message is ok, but having a shader failure without any message is the worst case…

I'm thinking about the Xonotic guys who try to port their game on the daemon engine, they will have plenty of issues like that, how these cases must be handled?

How compatibility can be maintained without preventing the daemon engine to go ahead, what can be kept and what must be dropped?
How must be handled errors? Invalidate the whole shader or the faulty line? Is it possible to minimize the consequences?
How errors must be reported to the user? (see missing texture but no warning message #776)

@Amanieu (since you wrote the commit I use as an example) @Kangz (since you wrote many commits like this one), @gimhael (since you're the renderer guy), what do you think about these questions?

illwieckz · 2015-08-23T01:04:57Z

As an example, if I replace the return false; by a continue; at this line:

Unvanquished/daemon/src/engine/renderer/tr_shader.cpp

Line 2515 in 1ec73fc

return false;

on the current master branch, there is still a warning raised but…

I get that (current master patched with continue):

instead of that (current master):

So, the other issues (fog, skybox, tree) are not fixed, but the fallback works for the ground.

A good idea would be to have a special cvar for mappers, if you set this cvar to strict, the renderer shader interpreter acts like the -Werror C flag: an error is raised and the shader is invalidate, if this cvar is set to transitionnal, a warning is raised but the interpreter just skip the weird thing and continue the job with the other lines. It will help a lot people who want to port their existing games to the daemon engine: they will be able to ship their early releases relying on the transitional mode, even if they plan to fix the issues, also it will allow them to not lost already existing community content.

Note: the tyrant ghost in background is another issue.

illwieckz · 2015-08-23T07:09:11Z

And if in some case it's not possible to continue a shader parsing and return is the only solution, the message should be “ERROR” instead of “WARNING”.

illwieckz · 2016-10-12T21:22:38Z

Just to say on current git the fog is fixed:

Since #939, this an easy way to load the railgun map:

# install etmain data
pimi.sh etmain
# get basic layout
mkdir -p ~/.unvanquished/game/layouts/railgun
wget http://dl.illwieckz.net/b/unvanquished/bugs/etmain-regression/railgun/spawn.dat \
     -O ~/.unvanquished/game/layouts/railgun/spawn.dat
# run unvanquished
./daemon -pakpath ~/.etlegacy/etmain -set fs_legacypaks 1 +devmap railgun spawn

(get pimi there, that's a tool to download Wolf:ET paks)

When an unknown keyword is found in a shader, the parser displays a warning, drops the current line but returns false, invalidating the whole shader, which is not a warning behavior. This change makes the shader parsing continue after the warning. If the shader must not continue, the parser must not raises a warning but an error.

…hed#799 When an unknown keyword is found in a shader, the parser displays a warning, drops the current line but returns false, invalidating the whole shader, which is not a warning behavior. This change makes the shader parsing continue after the warning. If the shader must not continue, the parser must not raises a warning but an error.

…hed#799 When an unknown keyword is found in a shader, the parser displays a warning, drops the current line but returns false, invalidating the whole shader, which is not a warning behavior. This change makes the shader parsing continue after the warning. If the shader must not continue, the parser must not raises a warning but an error. It was the behavior before the refactoring of this part.

illwieckz · 2016-10-12T21:52:17Z

See #966

When the warning does not invalidate the shader, this is how the map is rendered on daemon:

There is still an issue on the tree texture (looks like an untextured object without fallback). Same for that little object on the foreground.

illwieckz · 2016-10-12T23:17:38Z

For the tree texture issue, they look like nothing is rendered:

I chose to study this particular shader:

The shader code is:

textures/alpha/truss_m06a
{
    cull disable
    nomipmaps
    nopicmip
    surfaceparm nomarks
    surfaceparm nonsolid
    surfaceparm alphashadow
    surfaceparm metalsteps
    surfaceparm pointlight
    surfaceparm trans
    implicitMask -
}

I suspect the implicitMask code.

When an unknown keyword is found in a shader, the parser displays a warning, drops the current line but returns false, invalidating the whole shader, which is not a warning behavior. This change makes the shader parsing continue after the warning. If the shader must not continue, the parser must not raises a warning but an error. It was the behavior before the refactoring of this part.

Viech · 2017-01-02T11:59:12Z

@illwieckz does 2ad2583 fix this?

illwieckz · 2017-01-02T12:12:59Z

I will try next week but from my memories it only fixes the ground texturing, not the tree texturing.

illwieckz · 2017-09-08T04:39:33Z

The “disappearing tree” bug was introduced in commit e570876.

Badly this commit also introduced a bug that makes dæmon segfaults at map load. So you need commit 98ea7a9 to be able to run the game. So the bug can also in that other commit or a combination of these twos.

git checkout 6dbb61db05cdc76470c95271ea65edd1789c6519
make -j$(nproc)
./daemon

It works

git checkout e570876a40d824256532822d5ba7cb283e34f911
git cherry-pick 98ea7a986339ff6e8ebdc9d46064f40ba17de0f4
make -j$(nproc)
./daemon

It does not work

illwieckz · 2017-09-08T04:56:26Z

before:
after:
before:
after:

illwieckz · 2017-09-08T05:25:12Z

I verified that the "falling snow" effect was already missing in unvanquished 0.6.0 (the older build I have) so it probably wasn't there at all. It would be cool to know if it's a bug like the missing optional keyword invalidating the whole shader or if it's because of an unsupported feature. There is many nice snow/rain effects in some wolfet or quakelive maps that does not work on dæmon, but that does not looks like a regression.

Viech · 2017-11-20T13:38:00Z

@illwieckz can you compress this issue (that is, state briefly what is left to do at this point and update the issue title to reflect that)?

illwieckz · 2020-02-05T12:02:57Z

So, after the deep rewrite of many things in the renderer the last months, all the regressions reported in this thread are now fixed.

No one will never know what was broken and no one cares, some large pieces of code were simply rewritten from scratch in a way it sounds correct and original pieces were just nuked without any regret.

There is still be some bugs remaining, like some translucent texture being rendered above the fog (maybe related to DaemonEngine/Daemon#77 ?):

Or some edges going the void way, that may be related to fog too, as it looks like there is small offset between fog and non-fog:

But those are other bugs. So this issue can be closed. Yipiie.

illwieckz · 2020-02-10T10:36:04Z

The fog offset issue was fixed by DaemonEngine/Daemon#287

The only remaining bugs in that map are:

the translucent stalactite texture being rendered above the fog.
missing snowflakes particle (that's a game feature).

Viech added A-Renderer T-Verbosity labels Oct 23, 2015

illwieckz mentioned this issue Oct 26, 2015

missing texture but no warning message #776

Open

illwieckz mentioned this issue Oct 12, 2016

do not abort shader parsing on unknown keyword warning, ref #799 #966

Merged

illwieckz mentioned this issue Aug 28, 2018

partial void effect on station15 tree model DaemonEngine/Daemon#119

Open

illwieckz closed this as completed Feb 5, 2020

illwieckz added the T-Regression for things that used to work but don't anymore label Feb 5, 2020

illwieckz mentioned this issue Feb 10, 2020

renderer: fix infuriating grey line DaemonEngine/Daemon#287

Merged

illwieckz mentioned this issue Apr 12, 2020

The need for legacy vertex lighting: lightgrid does not mix well with lightmap (maps compiled with -aprox) DaemonEngine/Daemon#313

Open

illwieckz mentioned this issue Apr 12, 2020

Translucent texture rendered above fog DaemonEngine/Daemon#314

Open

some pending renderer regressions introduced in alpha 38 and alpha 40 #799

some pending renderer regressions introduced in alpha 38 and alpha 40 #799

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Oct 12, 2016 •

edited

illwieckz commented Oct 12, 2016

illwieckz commented Oct 12, 2016

Viech commented Jan 2, 2017

illwieckz commented Jan 2, 2017 via email •

edited

illwieckz commented Sep 8, 2017

illwieckz commented Sep 8, 2017

illwieckz commented Sep 8, 2017

Viech commented Nov 20, 2017

illwieckz commented Feb 5, 2020 •

edited

illwieckz commented Feb 10, 2020 •

edited

some pending renderer regressions introduced in alpha 38 and alpha 40 #799

some pending renderer regressions introduced in alpha 38 and alpha 40 #799

Comments

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 22, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Aug 23, 2015

illwieckz commented Oct 12, 2016 • edited

illwieckz commented Oct 12, 2016

illwieckz commented Oct 12, 2016

Viech commented Jan 2, 2017

illwieckz commented Jan 2, 2017 via email • edited

illwieckz commented Sep 8, 2017

illwieckz commented Sep 8, 2017

illwieckz commented Sep 8, 2017

Viech commented Nov 20, 2017

illwieckz commented Feb 5, 2020 • edited

illwieckz commented Feb 10, 2020 • edited

illwieckz commented Oct 12, 2016 •

edited

illwieckz commented Jan 2, 2017 via email •

edited

illwieckz commented Feb 5, 2020 •

edited

illwieckz commented Feb 10, 2020 •

edited