library_webgpu.js shim doesn't seem to support closure pass

[floooh]

emsdk version 3.1.46

Building and running webgpu samples with webgpu.h works fine without the closure pass, but when trying to enable closure with the linker option --closure 1, linking appears to work, but when running I get the following errors on the browser js console:

Uncaught (in promise) TypeError: Failed to execute 'configure' on 'GPUCanvasContext': Failed to read the 'format' property from 'GPUCanvasConfiguration': Required member is undefined.
    at Da (triangle-sapp.js:54:9)
    at triangle-sapp.wasm:0x1165
    at triangle-sapp.wasm:0x1b7b
    at triangle-sapp.js:42:369
    at mc (triangle-sapp.js:27:301)
    at triangle-sapp.js:42:252
onerror: Uncaught TypeError: Failed to execute 'configure' on 'GPUCanvasContext': Failed to read the 'format' property from 'GPUCanvasConfiguration': Required member is undefined.

triangle-sapp.js:54 Uncaught TypeError: Failed to execute 'configure' on 'GPUCanvasContext': Failed to read the 'format' property from 'GPUCanvasConfiguration': Required member is undefined.
    at Da (triangle-sapp.js:54:9)
    at triangle-sapp.wasm:0x1165
    at triangle-sapp.wasm:0x3269
    at Object.lb (triangle-sapp.js:18:70)
    at b (triangle-sapp.js:11:46)

The JS file location triangle-sapp.js:54:9 looks like this:

The only place in library_webgpu.js which has a configure call seems to be:

emscripten/src/library_webgpu.js

Lines 2607 to 2614 in 1311f22

	var configuration = {
	"device": device,
	"format": WebGPU.TextureFormat[
	{{{ gpu.makeGetU32('descriptor', C_STRUCTS.WGPUSwapChainDescriptor.format) }}}],
	"usage": {{{ gpu.makeGetU32('descriptor', C_STRUCTS.WGPUSwapChainDescriptor.usage) }}},
	"alphaMode": "opaque",
	};
	context["configure"](configuration);

I don't know what's needed to make such a JS shim closure-compatible, e.g. whether there are generally some sort of annotations missing, or only in this specific location. I could probably help with investigation and fixing if I would know where to start :)

Cheers!

(cc: @kainino0x)

[kripken]

Maybe add a little logging - is the value sent into format undefined somehow? If the read on the typed array is out of bounds, it would be, but it does seem odd for closure to cause that... if it is undefined, comparing to a non-closure build could help.

Otherwise, that code (both in the output and the source) seems ok.

The only other idea I'd have is if the error is actually on the internals of that object send into device:, a (if it's expected to have a format field but doesn't - logging out that object might help).

[floooh]

I'll try to investigate, thanks for the hints!

[kainino0x]

Most likely {{{ gpu.makeGetU32('descriptor', C_STRUCTS.WGPUSwapChainDescriptor.format) }}} -> z[c + 12 >> 2] is somehow producing a value that's either 0 or out of bounds of WebGPU.TextureFormat. This could be flaky if the C value was uninitialized, though I have no idea why this would be affected by Closure.

[floooh]

Hmm, the incoming WGPUSwapChainDescriptor looks correct (usage, width, height) except for the format which is 0.

...I'm getting that pixel format via wgpuSurfaceGetPreferredFormat, maybe this returns zero when closure is active, looking at that next...

[floooh]

I think I found the culprit... not sure yet why it happens. But:

Closure-ized wgpuSurfaceGetPreferredFormat (

emscripten/src/library_webgpu.js

Lines 2577 to 2580 in 1311f22

	wgpuSurfaceGetPreferredFormat: (surfaceId, adapterId) => {
	var format = navigator["gpu"]["getPreferredCanvasFormat"]();
	return WebGPU.PreferredFormat[format];
	},

) looks like this:

    qa: ()=>{
        var a = navigator.gpu.getPreferredCanvasFormat();
        return ob[a]
    }

'var a' is 'bgra8unorm' which is then used as key on the ob object (which should be a mapping from string names to numbers), but the ob property names have been minified:

So ob['bgra8unorm'] would return undefined, because there's no property bgra8unorm on ob, because those have been minified by closure.

[floooh]

Hmm, those string-to-number mapping objects all look the same, I assume those are all affected by the minification, but the PreferredFormat is just the first one that's encountered:

emscripten/src/library_webgpu.js

Lines 317 to 330 in 1311f22

	// Map from enum string back to enum number, for callbacks.
	DeviceLostReason: {
	'undefined': 0,
	'destroyed': 1,
	},
	PreferredFormat: {
	'rgba8unorm': 0x12,
	'bgra8unorm': 0x17,
	},
	BufferMapState: {
	'unmapped': 0,
	'pending': 1,
	'mapped': 2,
	},

I guess those need to be annotated somehow so that closure leaves the keys alone. I'll see if I can find any similar mapping objects elsewhere and see how those are handled.

PS: hmm apparently putting the property names into quotes should be enough to prevent closure minification... why doesn't this work? (PS: apparently only in 'ADVANCED' closure mode, I'll keep digging)

[floooh]

Ok I give up. I haven't found a way so far sneaking those string-to-number mappings past Closure.

Apparently there's an undocumented tag /** @expose */, and there's also an official tag /** @export */. Either they don't work as I would expect, or maybe comments are removed before closure sees them.

I also tried to create the mapping object differently like this:

    PreferredFormat: (() => {
      const map = {};
      map['rgba8unorm'] = 0x12;
      map['bgra8unorm'] = 0x17;
      return map;
    })(),

...also didn't seem to work.

Halp!!1 :)

[kripken]

Interesting... I tested various combinations here:

addToLibrary({
  $libA: {
    PreferredFormat: {
      'rgba8unorm': 0x12,
      'bgra8unorm': 0x17,
    },
  },
  $libB: {
    'rgba8unorm': 0x12,
    'bgra8unorm': 0x17,
  },
  $libC: function() {
    return {
      'rgba8unorm': 0x12,
      'bgra8unorm': 0x17,
    };
  }
});

Those show up in the output JS like this:

var libA = {
 PreferredFormat: {
  rgba8unorm: 18,
  bgra8unorm: 23
 }
};

var libB = {
 rgba8unorm: 18,
 bgra8unorm: 23
};

function libC() {
 return {
  "rgba8unorm": 18,
  "bgra8unorm": 23
 };
}

So the first two are broken (missing ") while the last is ok. The last is by far the most common way we use quoted objects, that is, inside code and not on an object that is directly included itself, so I'd guess something in that direct inclusion path is going wrong. I'm investigating that now.

[floooh]

Hmm, one solution is to turn the mapping into a function:

    mapPreferredFormat: (fmt) => {
      switch (fmt) {
        case 'rgba8unorm': return 0x12;
        case 'bgra8unorm': return 0x17;
      }
    },

...this allows it to get past the swapchain creation.

But then the next problem (and something entirely different) is in the wgpuDeviceGetLimits shim:

emscripten/src/library_webgpu.js

Lines 712 to 727 in 4b33521

	wgpuDeviceGetLimits: (deviceId, limitsOutPtr) => {
	var device = WebGPU.mgrDevice.objects[deviceId].object;
	var limitsPtr = {{{ C_STRUCTS.WGPUSupportedLimits.limits }}};
	function setLimitValueU32(name, limitOffset) {
	var limitValue = device.limits[name];
	{{{ makeSetValue('limitsOutPtr', 'limitsPtr + limitOffset', 'limitValue', 'i32') }}};
	}
	function setLimitValueU64(name, limitOffset) {
	var limitValue = device.limits[name];
	{{{ makeSetValue('limitsOutPtr', 'limitsPtr + limitOffset', 'limitValue', 'i64') }}};
	}
	setLimitValueU32('maxTextureDimension1D', {{{ C_STRUCTS.WGPULimits.maxTextureDimension1D }}});
	setLimitValueU32('maxTextureDimension2D', {{{ C_STRUCTS.WGPULimits.maxTextureDimension2D }}});
	setLimitValueU32('maxTextureDimension3D', {{{ C_STRUCTS.WGPULimits.maxTextureDimension3D }}});
	setLimitValueU32('maxTextureArrayLayers', {{{ C_STRUCTS.WGPULimits.maxTextureArrayLayers }}});

...this gets minified to:

ga: (a,b)=>{
        function c(g, h) {
            B[b + (8 + h) >> 2] = f.nb[g]
        }
        function d(g, h) {
            g = f.nb[g];
            Ga = [g >>> 0, (H = g,
            1 <= +Math.abs(H) ? 0 < H ? +Math.floor(H / 4294967296) >>> 0 : ~~+Math.ceil((H - +(~~H >>> 0)) / 4294967296) >>> 0 : 0)];
            B[b + (8 + h) >> 2] = Ga[0];
            B[b + (8 + h + 4) >> 2] = Ga[1]
        }
        var f = S.Wa[a].object;
        c("maxTextureDimension1D", 0);
        c("maxTextureDimension2D", 4);
        c("maxTextureDimension3D", 8);

Note how the original code:

    function setLimitValueU32(name, limitOffset) {
      var limitValue = device.limits[name];
      {{{ makeSetValue('limitsOutPtr', 'limitsPtr + limitOffset', 'limitValue', 'i32') }}};
    }

...minified to:

        function c(g, h) {
            B[b + (8 + h) >> 2] = f.nb[g]
        }

...meaning .limits is now .nb!

This might turn out to become a real rabbit hole...

[kripken]

Unfortunately the issue is due to how JS parsing and toString works:

> x = (function a() { return { 'b': 5 } })
[Function: a]
> x.toString()
"function a() { return { 'b': 5 } }"
> y = { 'b': 5 }
{ b: 5 }
> y.toString()
'[object Object]'

That is, JS engines keep a copy of the source JS for function, and toString() retrieves that including any quotation marks. But JS engines do not do that for objects, where the quotes are simply lost.

A hard way to fix this would be to not use the JS engine native parsing and instead use Acorn, whose AST does note quoted properties. But that would mean integration Acorn in an entirely new part of Emscripten, and could make compilation slower.

Another option might be to always quote object properties. But that would be excessive and increase code size.

Other than that, we could work around this in library_webgpu using a function as @floooh said. That would also increase code size a little, but just here. That might be the best option if no one else has a better idea.

[floooh]

That last format seems to work if the shim is changed like this:

    PreferredFormat: () => {
      return {
        'rgba8unorm': 0x12,
        'bgra8unorm': 0x17,
      };
    },
    // ...
    wgpuSurfaceGetPreferredFormat: (surfaceId, adapterId) => {
      var format = navigator["gpu"]["getPreferredCanvasFormat"]();
      return WebGPU.PreferredFormat()[format];
    },

...interestingly Closure seems to turn that function into:

    qa: ()=>({
        rgba8unorm: 18,
        bgra8unorm: 23
    })[navigator.gpu.getPreferredCanvasFormat()],

...pretty crazy...

PS: this works too:

    PreferredFormat: () => ({
      'rgba8unorm': 0x12,
      'bgra8unorm': 0x17,
    }),

[kripken]

...meaning .limits is now .nb!

If .limits arrives from the WebGPU API, then device.limits should be device['limits'] in library_webgpu.js.

[floooh]

Right. I guess somebody "just" needs to go through library_webgpu.js and fix all the little things.

On my side, one problem is that I don't have an exhaustive test code base which calls all WebGPU functions (and thus would catch all problems), e.g. I could only fix the stuff that's called in my code base, and I'm only using a subset of the WebGPU feature set.

But I can try to get my code working and provide a list of required changes (even if incomplete), or some sort of proposal PR.

[floooh]

Regarding inefficiency when using a function for the string-to-number mapping, I think closure takes care of this, e.g. by turning this:

  wgpuSurfaceGetPreferredFormat: (surfaceId, adapterId) => {
    var format = navigator["gpu"]["getPreferredCanvasFormat"]();
    return WebGPU.PreferredFormat()[format];
  }

...into this:

    qa: ()=>({
        rgba8unorm: 18,
        bgra8unorm: 23
    })[navigator.gpu.getPreferredCanvasFormat()],

...that extra mapping function call is removed!

(it might be slightly less efficient when not using closure though)

[floooh]

Looks like at least for my code, the changes are just in two categories (e.g. I currently have a state that works for me with closure minification):

All the string-to-number mappings need to be changed like this:

    PreferredFormat: () => ({
      'rgba8unorm': 0x12,
      'bgra8unorm': 0x17,
    }),

...and called like this (note that this is a function call now):

    WebGPU.PreferredFormat()[format];

And then there's a handful of places where WebGPU object properties are directly accessed, like:

    device.limits[name];

...needs to be changed into:

    device['limits'][name];

Cases I found:

• device.features
• device.limits
• device.label

...but it's not just device, but also:

• querySet.type
• querySet.label
• queue.label

...and probably more.

[sbc100]

And then there's a handful of places where WebGPU object properties are directly accessed, like:

    device.limits[name];

...needs to be changed into:

    device['limits'][name];

In these cases (where possible) it would be better to rely on closure extern files. The Web APIs and the objects they return as supposed to be known to closure so it should not minify them. In this case maybe the extern type definitions are just not up-to-date with the WebGPU Web API.

[sbc100]

In some cases we work around the "object evaluation + serialization removes quotes" issue by using __postset. See ERRNO_CODES :

emscripten/src/library.js

Line 1255 in 4b33521

$ERRNO_CODES__postset: `ERRNO_CODES = {

[kainino0x]

All the string-to-number mappings need to be changed like this:

The Closure builds definitely used to work with the current pattern for string-to-number mappings. Is this a new problem with Closure?

[floooh]

I just remember that there was an Emscripten SDK version which suddenly spammed a lot of Closure warnings, that's why I had to add awkward hints like this into my EM_JS() embeddings:

https://github.com/floooh/sokol/blob/d98c8b92c25070f13d0491f5fade1d9d2ca885ad/sokol_app.h#L4847

From looking at git blame, this must have been around October 2022.

Not sure if this matters for this specific problem, but I think there was either a Closure version update, or some reworking how the Closure pass works.

PS: Also the problem is actually quite esoteric I think, it only affects WebGPU functions which return enum values, and I think apart from getting the preferred framebuffer pixel format (which I also just added quite recently to my code), those calls are quite rare. And linking works perfectly fine, the problem only manifests at runtime when such a function is actually called.

[sbc100]

Yes, I think the increase in closure warnings was just due to a change to not hide them by default.

The issue with object literals as library elements is not new.. its always been an issue AFAIK.

[kainino0x]

PS: Also the problem is actually quite esoteric I think, it only affects WebGPU functions which return enum values, and I think apart from getting the preferred framebuffer pixel format (which I also just added quite recently to my code), those calls are quite rare. And linking works perfectly fine, the problem only manifests at runtime when such a function is actually called.

Good point, you're right. I think that would explain it. It's pretty likely no one has really noticed those were broken all along in Closure builds.

[kainino0x]

@floooh reports we still have issues here:

Failed to execute 'configure' on 'GPUCanvasContext': Failed to read the 'format' property from 'GPUCanvasConfiguration': Required member is undefined.

Not sure why I haven't seen this; I certainly have tried calling configure().

[kainino0x]

cc @shrekshao @lokokung

[floooh]

This is with emsdk 3.1.63 btw, I'll try to find out more later today.

[floooh]

The minified JS code of the function where that error happens looks like this:

    na: (a,b,c)=>{
        a = Q.get(a);
        b = ic.get(b);
        var d = [A[c + 16 >> 2], A[c + 20 >> 2]];
        0 !== d[0] && (b.canvas.width = d[0]);
        0 !== d[1] && (b.canvas.height = d[1]);
        b.configure({
            device: a,
            format: fc[A[c + 12 >> 2]],
            usage: A[c + 8 >> 2],
            alphaMode: "opaque"
        });
        return jc.create(b)
    }

...the b.configure() call fails, because configure doesn't exist (I assume it has been minified). PS: wrong, the error message says something different...

...and this seems to be the code without minification (which works fine)...

var _wgpuDeviceCreateSwapChain = (deviceId,surfaceId,descriptor)=>{
    var device = WebGPU.mgrDevice.get(deviceId);
    var context = WebGPU.mgrSurface.get(surfaceId);
    var canvasSize = [HEAPU32[descriptor + 16 >> 2], HEAPU32[descriptor + 20 >> 2]];
    if (canvasSize[0] !== 0) {
        context["canvas"]["width"] = canvasSize[0]
    }
    if (canvasSize[1] !== 0) {
        context["canvas"]["height"] = canvasSize[1]
    }
    var configuration = {
        device: device,
        format: WebGPU.TextureFormat[HEAPU32[descriptor + 12 >> 2]],
        usage: HEAPU32[descriptor + 8 >> 2],
        alphaMode: "opaque"
    };
    context.configure(configuration);
    return WebGPU.mgrSwapChain.create(context)
}

[floooh]

...ok when looking at the error message again:

TypeError: Failed to execute 'configure' on 'GPUCanvasContext': Failed to read the 'format' property from 'GPUCanvasConfiguration': Required member is undefined.

...it's the same error I wrote the ticket originally for...

...and when looking at the minified and original code, the only way this can happen is if this minified array index expression result in undefined:

fc[A[c + 12 >> 2]],

...which is the minified form of:

WebGPU.TextureFormat[HEAPU32[descriptor + 12 >> 2]],

[floooh]

...looking through the minified js shim I find that fc array, so that one seems to look correct...

fc = [, "r8unorm", "r8snorm", "r8uint",  ..... // lot's more pixel format following here

PS: A also is correct (it's the UInt32Array view on the heap)

...I'll check if I can find out anything with step-debugging...

[floooh]

Ok we're getting somewhere :)

When hovering the whole minified expression fc[A[c + 12 >> 2]], the result is indeed undefined.

The inner HEAPU32 lookup A[c + 12 >> 2] results in 0. But looking at the array fc[] there is no element at index 0 which explains the undefined result of the whole expression:

PS: for reference, the value of c is 553608 in the minified code....

[floooh]

...looking at the same code in the non-minified code now...

[floooh]

In the unminified code, the result of the HEAPU32 lookup HEAPU32[descriptor + 12 >> 2] is 23, and not 0 like in the minified version.

But I just realized why I had such a strong deja-vu all the time. I had that all figured out already 😂

#20415 (comment)

[floooh]

...e.g. the problem are these mapping objects:

emscripten/src/library_webgpu.js

Lines 398 to 417 in 87ef02c

	// Map from enum string back to enum number, for callbacks.
	Int_BufferMapState: {
	'unmapped': 0,
	'pending': 1,
	'mapped': 2,
	},
	Int_CompilationMessageType : {
	'error': 0,
	'warning': 1,
	'info': 2,
	},
	Int_DeviceLostReason: {
	'undefined': 1,
	'unknown': 1,
	'destroyed': 2,
	},
	Int_PreferredFormat: {
	'rgba8unorm': 0x12,
	'bgra8unorm': 0x17,
	},

...if you follow the discussion starting at #20415 (comment) there are various potential solutions (for instance wrapping those mappings into functions) - e.g. the decimal 23 I'm seeing in the unminified version is that 0x17 in Int_PreferredFormat mapping.