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

Crash when try use texture from Unit8Array. #42

Closed
eXponenta opened this issue Jun 9, 2022 · 1 comment · Fixed by #43
Closed

Crash when try use texture from Unit8Array. #42

eXponenta opened this issue Jun 9, 2022 · 1 comment · Fixed by #43
Labels
bug Something isn't working

Comments

@eXponenta
Copy link
Contributor

eXponenta commented Jun 9, 2022

Some js native classes have a set method but not have a copy.
Map/Set/TypedArray (UInt8Array for example).
Ok, there are not reason handle a Map/Set as props because there are not API what use it, but should be way pass ArrayBuffer view.

This line try to use copy method that wrong for this case:
https://github.com/pmndrs/react-ogl/blob/main/src/utils.ts#L144

Possible fix:

// we should bypass set for array buffer, because it not resizable. You cant set a 4 values to array of 0. 
if (target?.set && !ArrayBuffer.isView(value)) {
      if (target.constructor.name === value.constructor.name && target.copy) {
        target.copy(value)
      } else if (Array.isArray(value)) {
        target.set(...value)
      } else {
        // Support shorthand scalar syntax like scale={1}
        const scalar = new Array(target.length).fill(value)
        target.set(...scalar)
      }
} else {

This sometime can be used for API's like a :

{ /* texture 2x2 filled to red */ }
<texture 
     width={2} 
     height={2} 
     image={new Uint8Array([255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255]} 
/>
@CodyJasonBennett
Copy link
Member

Published a fix in 0.4.3. This was a bit of a blindspot, but we have good coverage now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants