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

Texture: Added Source class. #22846

Merged
merged 5 commits into from
Feb 2, 2022
Merged

Texture: Added Source class. #22846

merged 5 commits into from
Feb 2, 2022

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Nov 16, 2021

Related issue: see #17949

Description

This PR adds a new class Source which represents the data source for a texture. This approach allows to identify unique data sources and avoid duplicate texture uploads.

The code represents the minimal implementation which is necessary to introduce this feature. It should be 100% backwards compatible. It's possible to enhance the implementation with specific data source classes per texture (e.g. DataSource, CompressedSource etc.) but it's not really necessary for the first step.

Note: Assigning an instance of THREE.Source to a texture constructor is problematic since textures do not have a common signature for their data definitions. Meaning this quickly breaks if you want to assign a source object to a data texture because this class describes its image data with three parameters. So instead of doing:

const texture = new THREE.DataTexture( data, width, height, THREE.RedFormat );

you would expect:

const texture = new THREE.DataTexture( source, THREE.RedFormat );

which obviously breaks the signature. At least it is a bit messy to process this use case in code.

The PR does not support this pattern right now so the code example from #17949 (comment) looks like so:

const source = new THREE.Source( image ); // source.data = image

const material = new THREE.MeshBasicMaterial();
material.map = new THREE.Texture();
material.map.source = source;
material.map.offset.x = 0.5;

@mrdoob
Copy link
Owner

mrdoob commented Jan 20, 2022

@Mugen87 Do you want to give this a try this month, or should move it to next month?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 20, 2022

I would prefer to put this in r138. Merging at the beginning of the month allows us to have more time for verification.

@mrdoob mrdoob modified the milestones: r137, r138 Jan 20, 2022
@Mugen87 Mugen87 marked this pull request as ready for review January 30, 2022 08:56
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 1, 2022

I think now would be a good time to merge this PR 😇 .

@Mugen87 Mugen87 mentioned this pull request Feb 1, 2022
@mrdoob mrdoob merged commit 68dcb92 into mrdoob:dev Feb 2, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 2, 2022

Aaaand it's merged. Yay!

sunag added a commit to sunag/three.js that referenced this pull request Feb 6, 2022
Mugen87 pushed a commit that referenced this pull request Feb 6, 2022
@0b5vr 0b5vr mentioned this pull request Feb 22, 2022
16 tasks
0b5vr added a commit to 0b5vr/three-ts-types that referenced this pull request Feb 22, 2022
I have very low confidence about this commit...

See: mrdoob/three.js#22846
See: mrdoob/three.js#23419
joshuaellis pushed a commit to three-types/three-ts-types that referenced this pull request Mar 1, 2022
* feature (Texture): Add Source class

I have very low confidence about this commit...

See: mrdoob/three.js#22846
See: mrdoob/three.js#23419

* fix: forgot to export `Source` I added in the previous commit

* test: add a test code, texture-source.ts

* fix: Source.data can be ImageData

* fix: make `source.data` be `any`

* fix (Texutre): Texture variantes, make `.image` an accessor
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
* Texture: Added Source class.

* WebGLTextures: Rebase.

* WebGLTextures: Clean up.
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
@Ben-Mack
Copy link

Ben-Mack commented May 8, 2022

Does this PR also decrease the memory usage?
If 1 texture is used in 2 material, so previously GPU has to contain 2 duplicated texture in the memory, and this PR will reduce the memory usage back to 1, is it correct?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 8, 2022

If 1 texture is used in 2 material, so previously GPU has to contain 2 duplicated texture in the memory,

I don't think this was true before, see https://jsfiddle.net/6hbsov02/1/.

The redundant upload did appear when cloning an existing texture, see https://jsfiddle.net/p93ca80j/.

This is now fixed with latest versions: https://jsfiddle.net/p93ca80j/2/

@osov
Copy link

osov commented May 20, 2022

Hello, in the new version (140) when creating a clone of a texture after using it, a warning appears:
GL_INVALID_OPERATION: Texture is immutable.
https://jsfiddle.net/hq8617d3/
If the texture is created without a pause, then there is no warning. Is this a bug or am I missing something?
https://jsfiddle.net/p93ca80j/2/

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 20, 2022

Thanks for reporting. This is actually a bug in WebGLTextures.

@Josema
Copy link

Josema commented Apr 21, 2023

Sorry but, what is image in this example, the docs do not provide any further information.

const source = new THREE.Source( image ); // source.data = image

const material = new THREE.MeshBasicMaterial();
material.map = new THREE.Texture();
material.map.source = source;
material.map.offset.x = 0.5;

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 21, 2023

Usually something that you load via ImageLoader.

@Josema
Copy link

Josema commented Apr 21, 2023

Is it possible to set the source after being loaded like we usually do with THREE.TextureLoader?
I'm trying to emulate this function:

function LoadTexture(url) {
    const TextureLoader = new THREE.TextureLoader()
    const texture = TextureLoader.load(url)
    return texture
}

To something like this:

function LoadTexture(url) {
    const texture = new THREE.Texture()
    const loader = new THREE.ImageLoader()
    loader.load(url, (image) => {
        texture.source = new THREE.Source(image)
    })
    return texture
}

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 21, 2023

Yes, but you have to set the needsUpdate flag on the texture to true after assigning a source.

@Josema
Copy link

Josema commented Oct 17, 2023

Is there a way to know when this task is done?

texture.source = mysource
texture.needsUpdate = true

I am using expo-three for react-native. But after a lot of hours of testing I had to use a setTimeout just to apply the same source to different textures. Does not work if I apply them in a regular for/loop. The texture does not show up.

My setTimeout solution feels very dirty. Just wondering if there is another aproach for this.

@donmccurdy
Copy link
Collaborator

@Josema the texture will upload to the GPU synchronously, the next time a material using it renders. If you'd like to force that to happen at a particular time, you can upload the texture in advance:

renderer.initTexture(texture);

https://threejs.org/docs/?q=renderer#api/en/renderers/WebGLRenderer.initTexture

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.

6 participants