maierfelix/nvk

VkImageBlit and VkImageBlit2KHR srcOffsets and dstOffsets cause RangeError crash.

Someoneamzing opened this issue · 0 comments

The Issue

When an instance of VkImageBlit or VkImageBlit2KHR is flushed, if the srcOffsets / dstOffsets VkOffset3Ds share the top struct's memory then the copy operation will crash with a RangeError (This example came from the srcOffsets in a VkImageBlit):

RangeError: offset is out of bounds
    at Uint8Array.set (<anonymous>)
    at VkImageBlit.flush (F:\Programming\misc\nvk\generated\1.2.162\win32\interfaces.js:15515:15)

Steps to Reproduce

Running this simple script can highlight the issue:

const blit = new VkImageBlit();
blit.srcOffsets[0].x = 1;
blit.srcOffsets[0].y = 1;
blit.srcOffsets[0].z = 1;
blit.flush();

Running this script as is will cause a RangeError crash as explained above.

However, the following script will run successfully, i.e. no range error crash (albeit wasting memory as explained below):

const blit = new VkImageBlit();
blit.srcOffsets[0] = new VkOffset3D();
blit.srcOffsets[1] = new VkOffset3D();
blit.dstOffsets[0] = new VkOffset3D();
blit.dstOffsets[1] = new VkOffset3D();
blit.flush();

What is known

From some quick digging it appears that when the offsets share the blit's memory the Uint8Array that is used to copy their data is not bounded by the offset struct's offset/length, meaning it copies the entire top-level structure. Due to the offset supplied to the set command this causes the set to attempt to write past the end of the buffer (hence the RangeError).

From a snippet inside the VkImageBlit's flush method: (lines 15495-15518)

  if (this._srcOffsets !== null) {
    let array = this._srcOffsets;
    
    if (array.length !== 2) {
      throw new RangeError("Invalid array length, expected length of '2' for 'VkImageBlit.srcOffsets'");
      return false;
    }
    for (let ii = 0; ii < array.length; ++ii) {
      
      if (!array[ii] || (array[ii].constructor !== VkOffset3D)) {
        throw new TypeError("Invalid type for 'VkImageBlit.srcOffsets[" + ii + "]': Expected 'VkOffset3D' but got '" + typeToString(array[ii]) + "'");
        return false;
      }
      if (!array[ii].flush()) return false;
    };
    
    let dstView = new Uint8Array(this.memoryBuffer);
    let byteOffset = 0x10;
    for (let ii = 0; ii < array.length; ++ii) {
      let srcView = new Uint8Array(array[ii].memoryBuffer); //<========= This line here
      dstView.set(srcView, byteOffset);
      byteOffset += VkOffset3D.byteLength;
    };
  }

It appears that the srcView should be created with an offset and length to match that of the VkOffset3D to prevent copying the whole top-level structure.

Temporary Workaround

For now simply setting these offset elements to new instances of VkOffset3D that do not share the top level memory solves the issue, however, this is inefficient in terms of memory as it requires extra to store the separated instances.

const blit = new VkImageBlit();
blit.srcOffsets[0] = new VkOffset3D(); // Here we set the element to a new instance of VkOffset3D that does not share the VkImageBlit's memory.
blit.srcOffsets[0].x = 0;
blit.srcOffsets[0].y = 0;
blit.srcOffsets[0].z = 0;
blit.srcOffsets[1] = new VkOffset3D(); //And here as well
blit.srcOffsets[1].x = mipWidth;
blit.srcOffsets[1].y = mipHeight;
blit.srcOffsets[1].z = 1;