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

A couple of bugs I found in the loader.c #33

Closed
sasmaster opened this issue Jul 4, 2017 · 7 comments
Closed

A couple of bugs I found in the loader.c #33

sasmaster opened this issue Jul 4, 2017 · 7 comments
Labels

Comments

@sasmaster
Copy link

1.When one loads an array texture, GL_TEXTURE_2D_ARRAY,in the loader.c it is bound to a default target which is probably GL_TEXTURE_2D.It is never bound to GL_TEXTURE_2D_ARRAY afterwards.It leads to INVALID OPERATION error when trying to rebound to GL_TEXTURE_2D_ARRAY later.Please fix it by binding the texture only after the correct target has been set. (lines 543 -555)

2.When loading a CUBE MAP,the loader iterates over the faces,right?That's fine.But then the texture target enum that the loader returns is not GL_TEXTURE_CUBE_MAP but the last face target enum.This one also leads to a crash when used by the caller to bind a target.
I fixed it like this:

```

if (errorCode == KTX_SUCCESS)
{

	if (texinfo.generateMipmaps && pfGlGenerateMipmap) {
		pfGlGenerateMipmap(texinfo.glTarget);
	}
    if (iscubemap)
    {
        *pTarget = GL_TEXTURE_CUBE_MAP;
	 
    }....
 
@sasmaster
Copy link
Author

I really wonder how nobody got on these before.Like,no one loads anything but textures 2D ? :P

@MarkCallow
Copy link
Collaborator

MarkCallow commented Jul 4, 2017

Thank you for the bug report. Please send me a PR with the fixes.

really wonder how nobody got on these before.Like,no one loads anything but textures 2D ? :P

The GL loadtests only test 2D textures but it is surprising nobody else spotted these errors. I'd be happy to accept a PR with additional tests.

The tests for the new Vulkan loader (WIP over on https://github.com/msc-/KTX) include array and cube texture tests.

@sasmaster
Copy link
Author

sasmaster commented Jul 4, 2017 via email

@MarkCallow
Copy link
Collaborator

I can drop the whole loader.c here if you wish.

If you have a fix for #1, please drop those lines here. No need for the whole thing.

@sasmaster
Copy link
Author

`
texnameUser = pTexture && *pTexture;
if (texnameUser) {
texname = *pTexture;
} else {
glGenTextures(1, &texname);
}

/* load as 2D texture if 1D textures are not supported */
if (texinfo.textureDimensions == 1 &&
    ((texinfo.compressed && (pfGlCompressedTexImage1D == NULL)) ||
     (!texinfo.compressed && (pfGlTexImage1D == NULL))))
{
    texinfo.textureDimensions = 2;
    texinfo.glTarget = GL_TEXTURE_2D;
    header.pixelHeight = 1;
}

if (header.numberOfArrayElements > 0)
{
    if (texinfo.glTarget == GL_TEXTURE_1D)
    {
        texinfo.glTarget = GL_TEXTURE_1D_ARRAY_EXT;
    }
    else if (texinfo.glTarget == GL_TEXTURE_2D)
    {
        texinfo.glTarget = GL_TEXTURE_2D_ARRAY;
    }
    else if (texinfo.glTarget == GL_TEXTURE_CUBE_MAP)
    {
        texinfo.glTarget = GL_TEXTURE_CUBE_MAP_ARRAY;
    }
    else
    {
        /* No API for 3D arrays yet */
        return KTX_UNSUPPORTED_TEXTURE_TYPE;
    }
    texinfo.textureDimensions++;
}
errorTmp = glGetError();
assert(errorTmp == 0);
glBindTexture(texinfo.glTarget, texname);
errorTmp = glGetError();
assert(errorTmp == 0);

`
Before that,the glBindTexture was at the top.

@MarkCallow MarkCallow added the bug label Jun 11, 2018
@MarkCallow
Copy link
Collaborator

This is fixed in the new code in branch incoming. I'll backport the fix to master soon.

MarkCallow added a commit that referenced this issue Jun 29, 2018
Fixes item #1 of issue #33. Item #2 did not occur in incoming.
MarkCallow added a commit that referenced this issue Jun 29, 2018
@MarkCallow
Copy link
Collaborator

Item #1 is now fixed in master. #2 was already fixed in master.

Both are also fixed in incoming.

MarkCallow added a commit to MarkCallow/KTX-Software that referenced this issue Feb 29, 2020
MarkCallow added a commit to MarkCallow/KTX-Software that referenced this issue Feb 29, 2020
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 21, 2024
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 21, 2024
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants