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

If "dimension" is "2d-array", 'WEBGLTexture' will not be initialized correctly. #1760

Closed
Seungup opened this issue Jul 5, 2023 · 4 comments · Fixed by #1765
Closed

If "dimension" is "2d-array", 'WEBGLTexture' will not be initialized correctly. #1760

Seungup opened this issue Jul 5, 2023 · 4 comments · Fixed by #1765
Assignees
Labels

Comments

@Seungup
Copy link
Collaborator

Seungup commented Jul 5, 2023

import { WEBGLTexture } from '@luma.gl/webgl'

const deivce = ...;

new WEBGLTexture(device, {
  dimension: '2d-array',
  width: width,
  height: height,
  depth: 0,
  data: null,
  format: 'rgba8uint',
  type: GL.UNSIGNED_BYTE,
  mipmaps: false,
  sampler: {
    addressModeU: 'clamp-to-edge',
    addressModeV: 'clamp-to-edge',
    magFilter: 'linear',
    minFilter: 'linear'
  }
});

image

I think we should add a case where the value of 'dimension' is '2d-array' to this part. like

export class WEBGLTexture extends Texture {
   ...

  setImageData(options: SetImageDataOptions) {
    if ( this.props.dimension === '3d' || this.props.dimension === '2d-array' ) {
        return this.setImageData3D(options);
    }
    ...
  }
  
  ...
}

https://github.com/visgl/luma.gl/blob/7ee95470a6a2f62753201beb0c36b00e37c70455/modules/webgl/src/adapter/resources/webgl-texture.ts#L442C41-L442C41

@Seungup
Copy link
Collaborator Author

Seungup commented Jul 5, 2023

I'm using the v9.0.0-alpha.16 version, but it looks like the same problem with alpha.16 will appear on v9.0.0-alpha.21. (No modifications made in versions 21 and 16)

@ibgreen
Copy link
Collaborator

ibgreen commented Jul 5, 2023

@Seungup thanks for reporting. I suspect your proposed fix is correct.

We should add the 2d-array case to the test suite. The WEBGLTexture class is due for a refactor and it will be a good time to do that.

PS - I am impressed that you are trying to use the WIP alpha releases from luma.gl. Are you interested in contributing to the project?

@Seungup
Copy link
Collaborator Author

Seungup commented Jul 6, 2023

@ibgreen Thank you for your response. I'm glad you agree with my suggested fix and the idea of adding new test cases.

I'm very excited about the potential of contributing to the luma.gl project. To give you a little more context, we're using the alpha versions of luma.gl in our actual company projects. The main reasons for this include the TypeScript support that these versions provide, along with lower overhead and the introduction of the shader module concept. These features were key factors in our decision to choose luma.gl over three.js, and they've allowed us to deeply engage with and contribute to the project.

I'm looking forward to your guidance on the next steps. Thank you!

@ibgreen
Copy link
Collaborator

ibgreen commented Jul 7, 2023

@Seungup

It is great to hear that you find value in luma.gl and its design philosophy. I do want you to have the right expectations if you are using luma.gl v9 for production work. luma.gl v9 is a big breaking release focused on introducing WebGPU and at least until we start publishing beta releases, it will be a work in progress. Happy to try to help you around any rough corners but there are limits to how much support we can provide on an alpha version. That said, if your goal is to work closely to the GPU and you are willing to do some fixes yourself, it may well work out for you.

Regardless, if possible, please join the deck.gl slack so that we can discuss directly. I have also sent you an invite to the luma.gl repository, so that you can push branches and create PRs directly instead of working through a fork.

@Seungup Seungup self-assigned this Jul 13, 2023
@Seungup Seungup added the bug label Jul 13, 2023
@Seungup Seungup assigned Seungup and unassigned Seungup Jul 13, 2023
Seungup added a commit that referenced this issue Jul 13, 2023
Modify to call "setImageData3D" method even if "2d-array" in addition to "3d"
ibgreen pushed a commit that referenced this issue Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants