WriteTexture doesn't need WGPUTextureDataLayout.offset

[kainino0x]

While looking at some code in Blink I noticed that in C, there is no point in having WGPUTextureDataLayout.offset in wgpuQueueWriteTexture. It's there because (1) it matches JS and (2) we use the offset in WGPUImageCopyBuffer. However in C, it's possible to just offset the data pointer instead, with ptr + offset or &ptr[offset] (where offset is now in array elements, instead of bytes).

In webgpu.h we could remove this redundancy by moving offset out of WGPUTextureDataLayout into WGPUImageCopyBuffer. Is that worthwhile?

[Kangz]

IMHO it's probably not worthwhile, while a bit weird, it matches the JS API more and doesn't really have a cost associated with it.

[kainino0x]

It already doesn't match super well (because we take a raw pointer + length instead of a typed array), but it does still match better.

The place where this causes friction is in wire implementations, where it looks like you should send span(data, dataSize) but really you should not do this. Like, should dawn wire just do what you asked and copy the range you gave? Or should wire be smarter and avoid copying the part before the offset (so that logic moves from blink into dawn wire)?

[Kangz]

That seems like an implementation detail of dawn::wire? We didn't do it in dawn::wire because we didn't have the texture format in there at the time and didn't want to move complicated logic there. But now we could.

That said it seems somewhat irrelevant to this discussion since that's an optimization under the as-if rule.

[kainino0x]

Tentatively closing as I'd like to go with no change, but leaving the label to discuss if we want to reopen.

[kainino0x]

Nov 21 meeting:

• KN: The offset just offsets the pointer, so it's redundant (you could just offset the pointer yourself). OTOH, it matches JS. Leave as is?
• KN: JS has this because it can't offset the pointer like C.
  • CF: Can't it, with a new TypedArray view?
  • KN: That's true. Those are surprisingly expensive though, reason why we have this offset (and others in WebGPU and WebGL) in the JS API at all. Wasm bindings point WebGL/WebGPU directly to whole heap and use pointer to offset it.
  • CF: Surprised, Rust-Wasm bindings typically do that a lot.
  • KN: Have benchmarks showing it is still bad. (here's the link to that: https://github.com/gpuweb/gpuweb/pull/4612#discussion_r1637417418 @cwfitzgerald)
• CF: Think we should not have it
• OK. KN to open PR

[kainino0x]

On that tangent, see also:

• https://github.com/gpuweb/gpuweb/issues/501
• https://github.com/KhronosGroup/WebGL/pull/1940

[kainino0x]

Nov 25 meeting:

• Verify this looks like what we wanted. Different from spec in two places (add offset, remove offset)
• KN/CF: (doesn't seem worth it to diverge from JS, would be nice if JS had put the offset as an argument to writeTexture, but it didn't)
• Don't change this.