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

Fix issue with copying to a texture from a HTMLVideoElement #12224

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Sep 30, 2024

Description

A HTMLVideoElement has additional videoHeight and videoWidth properties. These are the intrinsic dimensions. If we rely on the height and width values, we can get sub optimal values, and – as in the case of the reported bug – sometimes 0 values, which can break the WebGL operation entirely.

While I was in that code, I also opted to use naturalHeight and naturalWidth for HTMLImageElement, as those the intrinsic, not scaled, dimensions.

Issue number and link

Fixes #12219

Testing plan

  1. Visit the Video.html Sandcastle example
  2. Ensure the video is running on the sphere
  3. Open the development console and ensure no WebGL errors are reported

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

Copy link

Thank you for the pull request, @ggetz!

✅ We can confirm we have a CLA on file for you.

@ggetz
Copy link
Contributor Author

ggetz commented Sep 30, 2024

@jjspace Could you please review this fix for tomorrow's release?

Copy link
Contributor

@jjspace jjspace left a comment

Choose a reason for hiding this comment

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

Can confirm the sandcastle is now working again. Happy to see it was a small fix. Just had one small comment

packages/engine/Source/Renderer/Texture.js Outdated Show resolved Hide resolved
Co-authored-by: jjspace <8007967+jjspace@users.noreply.github.com>
@ggetz
Copy link
Contributor Author

ggetz commented Sep 30, 2024

Thanks @jjspace! Updated!

Copy link
Contributor

@jjspace jjspace left a comment

Choose a reason for hiding this comment

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

LGTM!

@jjspace jjspace merged commit 8016d9f into main Sep 30, 2024
9 checks passed
@jjspace jjspace deleted the fix-video-material branch September 30, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Texture WebGL: OUT_OF_MEMORY
2 participants