image-js / iobuffer

Read and write binary data in ArrayBuffers
http://image-js.github.io/iobuffer/
MIT License
43 stars 6 forks source link

Possible bug?: read arrays #61

Closed ghost closed 1 year ago

ghost commented 1 year ago

I think there was an overlook (sorry). When we slice the underlying array buffer and convert it to the TypedArray using new TypedArray(...) this won't work well depending on the Endianness, I believe. We may need to remove it.

Possibility for fixing ```typescript /** * Creates an array of corresponding to the type `type` and size `size`. * For example type `uint8` will create a `Uint8Array`. * @param size - size of the resulting array * @param type - number type of elements to read */ public readArray( size: number, type: T, ): InstanceType { const bytes = typedArrays[type].BYTES_PER_ELEMENT * size; const offset = this.byteOffset + this.offset; const slice = this.buffer.slice(offset, offset + bytes) if (this.littleEndian === this.hostBigEndian) { const slice = new Uint8Array(this.buffer.slice(offset, offset + bytes)); slice.reverse(); const returnArray = new typedArrays[type](slice.buffer); this.offset += bytes; returnArray.reverse(); return returnArray as InstanceType; } const returnArray = new typedArrays[type](slice); this.offset += bytes; return returnArray as InstanceType; } ```
Test ```typescript import { IOBuffer } from '../IOBuffer'; describe('read data', () => { // LE system stores 513 as [2, 1] // BE system as [1, 2] const dataFromLE = new Uint8Array([1, 2]); const dataFromBE = new Uint8Array([2, 1]); it('Compare Endian', () => { // little endian let theBuffer = new IOBuffer(dataFromLE); const LeRes = theBuffer.readArray(1, 'uint16'); expect(theBuffer.offset).toBe(2); //big endian theBuffer = new IOBuffer(dataFromBE); theBuffer.setBigEndian(); const BeRes = theBuffer.readArray(1, 'uint16'); expect(theBuffer.offset).toBe(2); expect(BeRes[0]).toBe(LeRes[0]); }); }); ```

(calculating the host endianness previously as a private property.)

With this code both all the tests and also netcdfjs pass the tests (when updated to use TypedArrays.) @targos

lpatiny commented 1 year ago

Woaw really interesting this double reverse approach !!!!

Just to have an idea I did a small jsBench: https://jsbench.me/oolb8dmqd7/1

And on a Float64Array we can achieve 4500 reverses of an array of 1 million element per seconds (so 2.25 Giga swap operations per seconds on my computer).

Yes this seems the way to go. Please in the testcase take care than the resulting array has at least 2 elements.

ghost commented 1 year ago

Didn't know about the site, thanks for the example and explaining it. I've tried to follow what you suggested in the current PR @lpatiny