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

Missing typecheck for GLTFExporter #26992

Closed
Pearces1997 opened this issue Oct 16, 2023 · 3 comments · Fixed by #27700
Closed

Missing typecheck for GLTFExporter #26992

Pearces1997 opened this issue Oct 16, 2023 · 3 comments · Fixed by #27700

Comments

@Pearces1997
Copy link

Pearces1997 commented Oct 16, 2023

Description

TypeError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The provided value is not of type '(CSSImageValue or HTMLCanvasElement or HTMLImageElement or HTMLVideoElement or ImageBitmap or OffscreenCanvas or SVGImageElement or VideoFrame)'.

Reproduction steps

  1. Go to https://www.filer.dev/convert/blend-to-glb
  2. Upload a blend file, convert it to glb or gltf
  3. GLTFExporter runs and throws an error, the convert fails if there is no image image textures it is undefined and fails the convert at ctx.drawImage(image, 0, 0)

Code

new GLTFExporter().parse(scene, result => {
        if (result instanceof ArrayBuffer) save(new Blob([result], { type: 'application/octet-stream' }), file_name; 
        else save(new Blob([JSON.stringify(result, null, 2)], { type: 'text/plain' }), file_name);
    }, (error) => {
        alert(error)
    }, { binary: type === 'glb', animations })

Scene is gotten from blend file, other converts work fine. Seems like a texture bug for GLTFExporter

Live example

[(https://www.filer.dev/convert/blend-to-glb)]

www.filer.dev uses threejs as well as other software to allow blender files to be rendered and converted in the web in an offline first manner.

Screenshots

ctx.drawImage(image, 0, 0)

Seems to run regardless if the image value is valid. Some type check can hopefully fix this is my hunch.

I tried this type of check in a ColladaExporter and it fixed my issue:

const iType = typeof image if (iType === 'CSSImageValue' || iType === 'HTMLCanvasElement' || iType === 'HTMLVideoElement' || iType === 'ImageBitmap' || iType === 'OffscreenCanvas' || iType === 'SVGImageElement' || iType === 'VideoFrame') ctx.drawImage(image, 0, 0)

Version

0.157.0 (latest)

Device

Desktop, Mobile

Browser

Chrome, Firefox, Safari, Edge

OS

Windows, MacOS, Linux, Android, iOS

@Pearces1997
Copy link
Author

@Pearces1997
Copy link
Author

Merged Pearces1997#1 into dev, will monitor for feedback.

Thanks!

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 17, 2023

TypeError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The provided value is not of type '(CSSImageValue or HTMLCanvasElement or HTMLImageElement or HTMLVideoElement or ImageBitmap or OffscreenCanvas or SVGImageElement or VideoFrame)'.

When the error occurs, I would say the exporter gets some kind of invalid input that has to be avoided on application level. However, I understand that making processImage() more robust is favorable so adding a similar check line in WebGLTextures would be good.

if ( ( typeof HTMLImageElement !== 'undefined' && image instanceof HTMLImageElement ) ||
( typeof HTMLCanvasElement !== 'undefined' && image instanceof HTMLCanvasElement ) ||
( typeof ImageBitmap !== 'undefined' && image instanceof ImageBitmap ) ) {

However, even with an additional check the exporter should throw an error so the application ensures valid input. You have not implemented this in your version but it should be done in this repo. Silently ignoring incompatible texture input can be very confusing for developers.

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

Successfully merging a pull request may close this issue.

2 participants