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

Support signed types #370

Closed
oeway opened this issue Feb 5, 2021 · 3 comments · Fixed by #372
Closed

Support signed types #370

oeway opened this issue Feb 5, 2021 · 3 comments · Fixed by #372

Comments

@oeway
Copy link
Contributor

oeway commented Feb 5, 2021

In some of the images, we have data type in <i2 and that generated an error in vizarr:

Error: Dtype not supported, must be u1, u2, u4, or f4

@manzt mentioned that it was possible to do data casting with geotiff (see here), and it will be great if we can have this generally available for vizarr as well.

@manzt
Copy link
Member

manzt commented Feb 5, 2021

Thanks for opening the issue! Yes, this should be feasible (we do cast int -> uint for geotiff). There are two approaches I see.

  • Duplicate the logic from TiffPixelSource to ZarrPixelSource and have the pixel-sources be responsible for adapting data for Viv layers.
  • Have the Viv layers cast data that isn't compatible, rather than the pixel sources.

I think the the latter is my preferred solution. The conversion logic would then be shared regardless of the pixel source. This way the sources shouldn't get out of sync. I'll take a look into adding this.

@ilan-gold
Copy link
Collaborator

Just seeing this but I think we can just update the shaders' handling without casting anything? Is there something blocking this that I'm missing (very possible)?

dtype === 'Float32' || isWebGL1 ? 'sampler2D' : 'usampler2D',

We just need to add isampler2D as an option to the above, I think, and then add the GL constants here (or we could refactor to move that line into the DTYPE_VALUES object):

viv/src/constants.ts

Lines 12 to 38 in a21b080

export const DTYPE_VALUES = {
Uint8: {
format: GL.R8UI,
dataFormat: GL.RED_INTEGER,
type: GL.UNSIGNED_BYTE,
max: 2 ** 8 - 1
},
Uint16: {
format: GL.R16UI,
dataFormat: GL.RED_INTEGER,
type: GL.UNSIGNED_SHORT,
max: 2 ** 16 - 1
},
Uint32: {
format: GL.R32UI,
dataFormat: GL.RED_INTEGER,
type: GL.UNSIGNED_INT,
max: 2 ** 32 - 1
},
Float32: {
format: GL.R32F,
dataFormat: GL.RED,
type: GL.FLOAT,
// Not sure what to do about this one - a good use case for channel stats, I suppose:
// https://en.wikipedia.org/wiki/Single-precision_floating-point_format.
max: 3.4 * 10 ** 38
}

@manzt
Copy link
Member

manzt commented Feb 5, 2021

Nope, sounds totally reasonable! I just assumed we needed to cast because that is what the tiff source did.

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