Emscripten command buffer error

[hugoam]

I took half a day to track down a nasty error that happened only when compiling to Emscripten + WebAssembly.

Apparently the problem is an alignment issue : in WebAssembly it seems you can't have commands aligned on 2 bytes, and it happens that the CreateImagePattern is the only one that has a total size that is not a multiple of 4. (it's 30 bytes large)

As a result, in WebAssembly, any call to CreateImagePattern completely messes up the rest of the command list.

Here is a raw git diff of a workaround to fix that, which is padding the command with another uint16_t. This is not a really nice solution, it's just a quick fix, so I suppose you can think of a nicer one (like forcing all commands to be padded to a multiple of 4 ?)

diff --git a/src/vg.cpp b/src/vg.cpp
index 79ab4d3..2a4a103 100644
--- a/src/vg.cpp
+++ b/src/vg.cpp
@@ -4242,6 +4242,9 @@ static void ctxSubmitCommandList(Context* ctx, CommandListHandle handle)
                        const float* params = (float*)cmd;
                        cmd += sizeof(float) * 5;
                        const ImageHandle img = CMD_READ(cmd, ImageHandle);
+#ifdef BX_PLATFORM_EMSCRIPTEN
+                       CMD_READ(cmd, uint16_t);
+#endif
                        ctxCreateImagePattern(ctx, params[0], params[1], params[2], params[3], params[4], img);
                } break;
                case CommandType::Text: {
@@ -5804,6 +5807,9 @@ static void clCacheRender(Context* ctx, CommandList* cl)
                        const float* params = (float*)cmd;
                        cmd += sizeof(float) * 5;
                        const ImageHandle img = CMD_READ(cmd, ImageHandle);
+#ifdef BX_PLATFORM_EMSCRIPTEN
+                       CMD_READ(cmd, uint16_t);
+#endif
                        ctxCreateImagePattern(ctx, params[0], params[1], params[2], params[3], params[4], img);
                } break;
                case CommandType::Text: {
@@ -6333,13 +6339,20 @@ static ImagePatternHandle clCreateImagePattern(Context* ctx, CommandList* cl, fl
 {
        VG_CHECK(isValid(image), "Invalid image handle");

+#ifdef BX_PLATFORM_EMSCRIPTEN
+       uint8_t* ptr = clAllocCommand(ctx, cl, CommandType::CreateImagePattern, sizeof(float) * 5 + sizeof(uint16_t) + sizeof(uint16_t));
+#else
        uint8_t* ptr = clAllocCommand(ctx, cl, CommandType::CreateImagePattern, sizeof(float) * 5 + sizeof(uint16_t));
+#endif
        CMD_WRITE(ptr, float, cx);
        CMD_WRITE(ptr, float, cy);
        CMD_WRITE(ptr, float, w);
        CMD_WRITE(ptr, float, h);
        CMD_WRITE(ptr, float, angle);
        CMD_WRITE(ptr, uint16_t, image.idx);
+#ifdef BX_PLATFORM_EMSCRIPTEN
+       CMD_WRITE(ptr, uint16_t, uint16_t(0));
+#endif

        const uint16_t patternHandle = cl->m_NumImagePatterns;
        cl->m_NumImagePatterns++;

[jdryg]

Thanks for the report. Will take a look probably tomorrow. Yes, padding all commands to some predefined alignment should be the way to go.

[jdryg]

I just pushed some changes regarding this bug.

The only way I could reproduce the crash was with -s SAFE_HEAP=1. Without it, WebAssembly (-s WASM=1) worked without a problem (as it's supposed to since it supports unaligned accesses), and asm.js (i.e. no WASM flag) asserted here https://github.com/jdryg/vg-renderer/blob/master/src/vg.cpp#L4368 But none of them crashed as they do with SAFE_HEAP=1.

Either way, please test it and report back as soon as you find the time.

[jdryg]

@hugoam Any news on this one? Does it still happen with the latest version of the code?

[hugoam]

Hey ! I might have confused WASM and asmjs when reporting the issue, it might have happened in the asmjs build. But it doesn't happen anymore in either of my builds, so I think your solution is perfect :)