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

Source: Add dataReady flag. #27649

Merged
merged 3 commits into from
Jan 30, 2024
Merged

Source: Add dataReady flag. #27649

merged 3 commits into from
Jan 30, 2024

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jan 30, 2024

Fixed #25133.

Description

This is an alternative implementation to #27572. It introduces a new flag Source.dataReady that allows to prevent the engine from uploading texture data. The texture will still be allocated though so it's possible to allocate the image/buffer and gradually fill/stream data into it.

This PR should also solve the use case mentioned in #27572 (comment).

Since the flag is true by default, it should not affect existing apps.

Copy link

github-actions bot commented Jan 30, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
673.6 kB (167.3 kB) 673.7 kB (167.3 kB) +94 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
454.4 kB (110.2 kB) 454.5 kB (110.2 kB) +94 B

@mrdoob
Copy link
Owner

mrdoob commented Jan 30, 2024

What do you think of naming it dataReady instead? (true by default too)

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 30, 2024

Sounds good as well! 👍

I'll update the PR.

@Mugen87 Mugen87 marked this pull request as ready for review January 30, 2024 11:05
@Mugen87 Mugen87 changed the title Source: Add uploadData flag. Source: Add dataReady flag. Jan 30, 2024
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 30, 2024

It seems after I have regenerated the E2E screenshot for webgl_camera, testes in different PRs fail. Regenerating it on my macOS system does not show a new diff so maybe someone else can update it?

@Mugen87 Mugen87 merged commit b909af7 into mrdoob:dev Jan 30, 2024
11 of 12 checks passed
@mrdoob mrdoob added this to the r161 milestone Jan 30, 2024
@mrdoob
Copy link
Owner

mrdoob commented Jan 30, 2024

@Mugen87 Done! 16c15ef

@Oletus
Copy link
Contributor

Oletus commented Jan 31, 2024

Sorry about the slow reply, this is great! I didn't get to integrating this yet, but this looks like it will solve our use case. Thank you!

@Oletus
Copy link
Contributor

Oletus commented Jan 31, 2024

One note how this could be improved: Maybe copyTextureToTexture functions should warn in case the dataReady flag of the source texture is set to false? People might think that the copy is happening from the buffer on the GPU, when it is in fact doing a texture upload:

this.copyTextureToTexture = function ( position, srcTexture, dstTexture, level = 0 ) {

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.

Setting up a texture without uploading initial data
3 participants