Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R_FindShader looks unnecessarily for filenames that does not exist then abort #806

Closed
illwieckz opened this issue Sep 8, 2015 · 7 comments
Assignees

Comments

@illwieckz
Copy link
Member

illwieckz commented Sep 8, 2015

Hi, experimenting with some Xonotic maps on Dæmon I experienced a shader bug with the Xonotic's Erbium map I already experienced with an old Tremulous map named Temple (formerly a Quake Ⅲ Arena map named Pyramid of the Magician). But unlike that very old Temple/PoM map, the Erbium map is a bleeding edge Xonotic map.

That is how looks this shader bug on Dæmon:

Erbium:
Erbium

Temple/PoM:
Temple/PoM

For Erbium, Dæmon complains like that:

Couldn't find image file for shader rocktomoss

For Temple, Dæmon complains like that:

Couldn't find image file for shader textures/sockter/ter_rockmoss

This is the Erbium's faulty shader (from scripts/map_erbium.shader file):

rocktomoss
{
        dpoffsetmapping - 2 match8 184
        q3map_baseShader textures/map_erbium/rocktomoss
        qer_editorimage textures/map_erbium/rocktomoss.jpg

        {
                map     textures/map_erbium/moss.jpg

        }
        {
                map     textures/map_erbium/rock-grey.jpg
                blendFunc GL_SRC_ALPHA GL_ONE_MINUS_SRC_ALPHA
                alphagen vertex
        }
        {
                map     $lightmap
                blendfunc GL_DST_COLOR GL_ZERO
                rgbGen identity

        }
}

This is the Temple's faulty shader (from scripts/sockter.shader file):

textures/sockter/ter_rockmoss   // Border/edge
{
        qer_editorimage textures/sockter/ter_rockmoss

        q3map_nonplanar
        q3map_shadeangle 120
        q3map_tcGen ivector ( 256 0 0 ) ( 0 256 0 )
        q3map_alphaMod dotproduct2 ( 0.0 0.0 0.75 )

        {
                map textures/sockter/ter_rock3  // Primary
                rgbGen identity
        }
        {
                map textures/sockter/ter_moss1  // Secondary
                blendFunc GL_SRC_ALPHA GL_ONE_MINUS_SRC_ALPHA
                alphaFunc GE128
                rgbGen identity
                alphaGen vertex
        }
        {
                map $lightmap
                blendFunc GL_DST_COLOR GL_ZERO
                rgbGen identity
        }
}

So, what's happening? I don't know exactly, but Dæmon looks for a file named like the shader name, even if it's useless. For example it looks for a file named rocktomoss or textures/sockter/ter_rockmoss and of course, it fails since they are not files.

The bug seems to happen in R_FindShader function, it does something like that:

shader_t       *R_FindShader( const char *name, shaderType_t type,
                  RegisterShaderFlags_t flags )
{
…
    COM_StripExtension3( name, strippedName, sizeof( strippedName ) );
…
    // ydnar: allow implicit mapping ('-' = use shader name)
    if ( implicitMap[ 0 ] == '\0' || implicitMap[ 0 ] == '-' )
    {
        Q_strncpyz( fileName, strippedName, sizeof( fileName ) );
    }
…
    if( !(bits & RSF_NOMIP) ) {
        image = R_FindImageFile( fileName, bits, FT_DEFAULT,
                     WT_REPEAT );
    } else {
        image = R_FindImageFile( fileName, bits, FT_LINEAR,
                     WT_CLAMP );
    }

    if ( !image )
    {
        ri.Printf( PRINT_DEVELOPER, "Couldn't find image file for shader %s\n", name );
        shader.defaultShader = true;
        return FinishShader();
    }
…
}

So, it calls R_FindImageFile on the shader's name, and return if there is no file named like that, even if it's useless.

@illwieckz
Copy link
Member Author

Note: I can share to any developer an Unvanquished-compatible and ready to use pk3 for the Temple map. It will be probably be better to use this one to track down this bug instead of the Erbium map since the Xonotic support still needs some tricks and the Temple map is expected to be fully compatible with Dæmon (Clean and compatible Tremulous map I already ported to last Unvanquished requirements), so the only bug in this map is this shader bug, so it's easier to track down it without dealing with plenty of other unrelated bugs that are still unknown and untracked.

@cmf028
Copy link
Member

cmf028 commented Sep 10, 2015

<- unv

https://github.com/ioquake/ioq3/blob/master/code/renderergl1/tr_shader.c#L2553 <- ioquake3 gl1
https://github.com/ioquake/ioq3/blob/master/code/renderergl2/tr_shader.c#L3174 <- ioquake3 gl2

The control flow here is the same if you take into account the "implicitMap" feature in our renderer. If the shader definition does not use "implicitBlend ", "implicitMask ", "implicitMap ", or "implicit" then we should be returning the shader returned by FinishShader() before reaching the point you are talking about. This is of course assuming that FindShaderInShaderText() is not returning nullptr.

@illwieckz
Copy link
Member Author

Hmm, I'm not sure to understand everything, but it is easy to fix?

If needed this is a map-temple pk3 (you need a res-tremulous pk3 too).

@illwieckz
Copy link
Member Author

illwieckz commented Feb 6, 2017

On current master, the warning message is gone for these textures, but are still not rendered.

I don't know if it's related, but on Erbium there is this shader:

textures/map_erbium/leaves2
{
    qer_editorimage textures/map_erbium/leaves2

	surfaceparm trans
	surfaceparm nonsolid
	polygonOffset
	cull none
	nopicmip
    {
        map $lightmap
        map textures/map_erbium/leaves2
        blendFunc GL_SRC_ALPHA GL_ONE_MINUS_SRC_ALPHA
        rgbgen identity
    }
}

And dæmon complains about that:

Warn: R_FindImageFile could not find image '$lightmaptextures/map_erbium/leaves2' in shader 'textures/map_erbium/leaves2' 

As you see, on this case, the two map calls are concatenated.

@DolceTriade
Copy link
Member

What if we move the two maps into different stages?

@illwieckz
Copy link
Member Author

It looks like the code in for-0.51.0 engine branch renders them nicely today. I don't know what fixed it but it looks fixed.

@illwieckz
Copy link
Member Author

illwieckz commented Mar 11, 2018

See:
Temple

Note that Erbium looks better too (there is others issues but these others issues do not look related to this particular bug).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants