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

TGALoader use DataTextureLoader #21377

Merged
merged 5 commits into from
Mar 1, 2021
Merged

TGALoader use DataTextureLoader #21377

merged 5 commits into from
Mar 1, 2021

Conversation

deepkolos
Copy link
Contributor

Description

use DataTexture may use less resource then canvas

@deepkolos deepkolos changed the title TGALoader use DataTextureLoader instead of cavnas TGALoader use DataTextureLoader Feb 28, 2021
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

Interesting idea! Do you mind modifying example/js/loaders/TGALoader.js instead and then generate the module via node utils/modularize.js(executed from the project's root directory).

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

Notice that the texture is now flipped:

https://raw.githack.com/deepkolos/three.js/master/examples/webgl_loader_texture_tga.html
https://threejs.org/examples/webgl_loader_texture_tga

When returning the texture data object in parse(), you have to set flipY to true. You also want to set minFilter to LinearMipmapLinearFilter.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

To make this work, it's also necessary to enhance DataTextureLoader so you can set generateMipmaps to true in the texture data object. Otherwise mipmaps are not generated.

@deepkolos
Copy link
Contributor Author

thanks to point out my mistake.

switch back to Loader, and use DataTexture but reset to Texture's default setting

before

after

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

I would prefer the usageDataTextureLoader. I maps better to how other loaders work which produce a data texture.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

Overriding load()in TGALoader should not be necessary if DataTextureLoader is enhanced at one place. But this can be refactored with a later PR.

@deepkolos
Copy link
Contributor Author

deepkolos commented Feb 28, 2021

if DataTextureLoader pass texture to the parse function, there is not need to Overriding load()

const texData = scope.parse( buffer, texture );

but not match parse meaning, it missing a step to custom the texture, the current way to do that is returning the setting by parse(), but the setting limited by DataTextureLoader

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

The return object in TGALoader.parse() should look like so:

return {
    data: imageData,
    width: header.width,
    height: header.height,
    flipY: true,
    minFilter: THREE.LinearMipmapLinearFilter,
    generateMipmaps: true
};

DataTextureLoader.parse() needs this enhancement then:

if ( texData.generateMipmaps !== undefined ) {

    texture.generateMipmaps = texData.generateMipmaps;

}

build/three.js Outdated Show resolved Hide resolved
@mrdoob mrdoob added this to the r127 milestone Mar 1, 2021
@mrdoob mrdoob merged commit be6a7db into mrdoob:dev Mar 1, 2021
@mrdoob
Copy link
Owner

mrdoob commented Mar 1, 2021

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants