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

TextureUtils: Add contain(), cover() and fill(). #28713

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 21, 2024

Related issue: #28512

Description

Since the original PR has not been touched for a couple of weeks and TextureUtils is now available, this PR implements contain(), cover() and fill() as helper functions.

@Mugen87 Mugen87 added this to the r166 milestone Jun 21, 2024
Copy link

github-actions bot commented Jun 21, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
681.6 kB (168.8 kB) 682.2 kB (169 kB) +532 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
459.5 kB (110.9 kB) 459.4 kB (110.9 kB) -18 B

@abernier
Copy link
Contributor

Ty for not procrastinating as much as I do 🙏

@Mugen87 Mugen87 merged commit 3ea97b9 into mrdoob:dev Jun 21, 2024
12 checks passed
@WestLangley
Copy link
Collaborator

WestLangley commented Jun 21, 2024

I think this implementation of contain() will have to be modified somehow, because as written, it will produce artifacts.
(I don't think setting wrapping would be a good solution.)
It would be nice if the canvas would show through around the edges...
https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit

Screenshot 2024-06-21 at 1 07 28 PM

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 21, 2024

I'm not sure this is possible. contains() might only be usable with certain images like the one from the example that have a single color at its border.

@WestLangley
Copy link
Collaborator

OK, all is good. I see that contain() can be made to work perfectly if the texture has a transparent background. 👍

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