-
-
Notifications
You must be signed in to change notification settings - Fork 35.4k
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
GLTFExporter: Add forcePowerOfTwoTexture option #13424
Conversation
@@ -459,21 +473,36 @@ THREE.GLTFExporter.prototype = { | |||
var mimeType = map.format === THREE.RGBAFormat ? 'image/png' : 'image/jpeg'; | |||
var gltfImage = { mimeType: mimeType }; | |||
|
|||
if ( options.embedImages ) { | |||
if ( options.embedImages || ( options.forcePowerOfTwoTexture && ! isPowerOfTwo( map.image ) ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this instead?
if ( options.embedImages ) {
var canvas = cachedCanvas = cachedCanvas || document.createElement( 'canvas' );
canvas.width = map.image.width;
canvas.height = map.image.height;
if ( options.forcePowerOfTwoTexture && ! isPowerOfTwo( map.image ) ) {
console.warn( 'GLTFExporter: image is not power of two. Resized and embedded.', image );
canvas.width = THREE.Math.floorPowerOfTwo( canvas.width );
canvas.height = THREE.Math.floorPowerOfTwo( canvas.height );
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, you meant you wanna POT conversion only if options.embedImages
is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... At least for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've updated. We might need to make clear that .forcePowerOfTwoTexture
option works only if .embedImages
is true by renaming the option or adding note to doc. Let's do that in another PR if necessary (sorry time to sleep here now!).
Haha, ok I believe in you. |
Thanks! |
#13397 (comment)
This PR adds
forcePowerOfTwoTexture
option toGLTFExporter
.