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

[webgl] Texture to tensor API. #4376

Merged
merged 39 commits into from
Feb 11, 2021
Merged

[webgl] Texture to tensor API. #4376

merged 39 commits into from
Feb 11, 2021

Conversation

annxingyuan
Copy link
Collaborator

@annxingyuan annxingyuan commented Dec 8, 2020

This fixes #4080
This fixes #3937

Changes

  • Add a new method to the API: tf.webgl.createTensorFromTexture that allows users to create tensors from textures.
  • Export methods in gpgpu_util that enumerate the texture properties for different environments (WebGL 1 vs 2, Float32 supported vs disabled, etc.)
  • Add method to the WebGL texture manager that allows registering an externally created texture to the internal registry of textures so it can be made available for reuse just like an internally create texture.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@google-cla google-cla bot added the cla: yes label Dec 8, 2020
@annxingyuan annxingyuan marked this pull request as ready for review December 14, 2020 14:07
Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Ann! It would be great to provide an example or examples of usage in the doccomment of createTensorFromTexture. As someone who is not very familiar with the use case, I have below questions:

  1. Is the use case to reuse a texture for other purpose, e.g. rendering, or is the use case to reuse the data in the texture for other purpose? What's the benefit, is it save data downloading time, is it reusing the canvas?
  2. Is the tensor created from the texture used as input for model or a copy of the output of model?
  3. Who owns the texture and who should dispose the texture, is it tfjs or the application or both (like registered in two places)?
  4. What does a comprehensive usage of this api look like?

It would be great to get an answer from the doccomment.

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @jinjingforever, @lina128, @pyu10055, and @tafsiri)


tfjs-backend-webgl/src/backend_webgl_test.ts, line 153 at r1 (raw file):

        'WEBGL_RENDER_FLOAT32_ENABLED', webglRenderF32EnabledFlagSaved);
    tf.env().set('WEBGL_PACK', webglPackedFlagSaved);
    tf.engine().startScope();

endScope?

Copy link
Collaborator Author

@annxingyuan annxingyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback Ping and Na. Please take another look.

Ping - regarding your question about physical shape requirements - I added some tests wherein physicalShape !== logicalShape, and I added a description in the docs about the constraints for physical shape. Let me know if any of it is unclear.

Na - regarding your questions about usage - I added some more detail to the docs - please let me know if any of it is still unclear.

Reviewable status: 0 of 1 approvals obtained (waiting on @jinjingforever, @lina128, @pyu10055, and @tafsiri)


tfjs-backend-webgl/src/backend_webgl_test.ts, line 153 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

endScope?

Done


tfjs-backend-webgl/src/webgl.ts, line 45 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

for these three params internalFormat, textureFormat and textureType, are there enum or type to restrict the values?

Done

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the doccomment Ann, LGTM. I have another question, is the second use case in the issue (#3937) (post-processing after tfjs computation, keep everything in gpu) fulfilled or will there be a separate PR for that or is that not feasible? #3937

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @jinjingforever, @lina128, @pyu10055, and @tafsiri)

@annxingyuan
Copy link
Collaborator Author

annxingyuan commented Dec 22, 2020

Hi Na - regarding the second use case of post-processing in TF.js without leaving the GPU, this is possible today as long as the model returns tensors. For example the faceLandmarksDetection model returns its prediction as a JS array by default, but it can also return its prediction as a tensor if the user passes in true for the returnTensors argument to estimateFaces.

In this case, the user can retrieve the texture backing the tensor by calling MathBackendWebGL.getTexture. This is what I'm doing in one of the newly introduced tests: https://github.com/tensorflow/tfjs/pull/4376/files#diff-9eafc72e191f4188b8e6556cb7f753645212ec3a97d0ed9f5d0fbcc97aa0f611R232

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the context Ann, this is great!

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @jinjingforever, @lina128, @pyu10055, and @tafsiri)

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tensor output, how do user get the texture handle for the tensor?

Reviewed 4 of 6 files at r1, 3 of 3 files at r2.
Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @jinjingforever, @lina128, and @tafsiri)

@annxingyuan
Copy link
Collaborator Author

@pyu10055 - users can retrieve the texture backing a tensor like so:

const tex = (tf.backend() as MathBackendWebGL).getTexture(someTensor.dataId);

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, is this documented somewhere, if not, would be good to add to jsdoc of this method.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @jinjingforever, @lina128, and @tafsiri)

@annxingyuan
Copy link
Collaborator Author

@pyu10055 good idea - done.

@annxingyuan
Copy link
Collaborator Author

@pyu10055 Hey Ping - sorry just getting back to this PR now. I'm trying to remember the changes we discussed making to this PR. Correct me if I'm wrong, but were we discussing adding functionality to fromPixels to accept a texture as input as well? If so, I was thinking about it more and it actually doesn't feel like that belongs in fromPixels, since the other acceptable inputs to fromPixels are more straightforwardly categorizable as images.

Let me know if you agree, or if there was something else we wanted to change about this PR. Otherwise I will merge it!

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Ann, I think this is fine. One question on texture retrieval for post processing, do users need to do unpacking specifically? If so, how does the users know if the tensor is packed in the texture? Thanks!

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @jinjingforever, @lina128, and @tafsiri)

@annxingyuan
Copy link
Collaborator Author

Hi @pyu10055 - this change should not affect the API for downloading textures. But to answer your question, the TextureData associated with each dataId in the WebGL backend contains a isPacked property.

@annxingyuan annxingyuan merged commit 65dcfcc into master Feb 11, 2021
@annxingyuan annxingyuan deleted the tex_to_tensor branch February 11, 2021 20:47
@markmckenna
Copy link

@annxingyuan thank you for this! We had planned to work on it but haven't been able to allocate time toward it thus far. Seeing it move into main hopefully prompts deeper interest here :)

@sebavan
Copy link

sebavan commented Feb 11, 2021

@annxingyuan This is great !!! thanks a lot

mattsoulanille added a commit to mattsoulanille/tfjs that referenced this pull request Feb 12, 2021
mattsoulanille added a commit that referenced this pull request Feb 12, 2021
@pyu10055
Copy link
Collaborator

cc @lina128

@NataliaDSmirnova
Copy link

sorry @pyu100552, I has moved my question to #3937.

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

Successfully merging this pull request may close these issues.

VRAM hosting of convolution output [webgl] allow direct WebGL tensor creation/retrieval using texture handle
6 participants