GaussianSplats3D icon indicating copy to clipboard operation
GaussianSplats3D copied to clipboard

Compatibility issues with rendering on iOS devices.

Open whwhwwhwwh opened this issue 1 year ago • 4 comments

Hello, after conducting my tests, there still seems to be an issue with iOS versions below 16.4. I think the following changes can be made to the code in the file /src/worker/SortWorker.js:

// iOS makes choosing the right WebAssembly configuration tricky :(
let iOSSemVer = isIOS() ? getIOSSemever() : null;
if (!enableSIMDInSort && !useSharedMemory) {
    sourceWasm = SorterWasmNoSIMD;
    if (iOSSemVer && iOSSemVer.major < 16) {
        sourceWasm = SorterWasmNoSIMDNonShared;
    }
} else if (!enableSIMDInSort) {
    sourceWasm = SorterWasmNoSIMD;
} else if (!useSharedMemory) {
    if (iOSSemVer && iOSSemVer.major < 16) {
        sourceWasm = SorterWasmNonShared;
    }
}

In the code, change the condition if (iOSSemVer && iOSSemVer.major < 16) { to if (iOSSemVer && iOSSemVer.major <= 16 && iOSSemVer.minor < 4) Moreover, according to the previous logic, the if inside the last else if should not be reachable.

Therefore, the modified code segment would be:

// iOS makes choosing the right WebAssembly configuration tricky :(
const iOSSemVer = isIOS() ? getIOSSemever() : null
if (!enableSIMDInSort && !useSharedMemory) {
  sourceWasm = SorterWasmNoSIMD
  if (iOSSemVer && iOSSemVer.major <= 16 && iOSSemVer.minor < 4)
    sourceWasm = SorterWasmNoSIMDNonShared
}
else if (!enableSIMDInSort) {
  sourceWasm = SorterWasmNoSIMD
}

whwhwwhwwh avatar Jun 25 '24 07:06 whwhwwhwwh

I can definitely change the version test to:

if (iOSSemVer && iOSSemVer.major <= 16 && iOSSemVer.minor < 4)

However the last existing else block is reachable when enableSIMDInSort is true, but useSharedMemory is false, so I'd like to keep it. It should really be:

} else if (!useSharedMemory) {
    sourceWasm = SorterWasmNonShared;
}

mkkellogg avatar Jun 25 '24 14:06 mkkellogg

Yes, I agree with your point, haha.

whwhwwhwwh avatar Jun 26 '24 02:06 whwhwwhwwh

I can definitely change the version test to:

if (iOSSemVer && iOSSemVer.major <= 16 && iOSSemVer.minor < 4)

However the last existing else block is reachable when enableSIMDInSort is true, but useSharedMemory is false, so I'd like to keep it. It should really be:

} else if (!useSharedMemory) {
    sourceWasm = SorterWasmNonShared;
}

After testing, I've discovered that modifying the code to:

} else if (!useSharedMemory) {
    sourceWasm = SorterWasmNonShared;
}

results in the following error when SharedMemory is disabled in desktop browsers:

error: 'WebAssembly.instantiate(): Import #0 "env" "memory…hared state of memory, declared = 0, imported = 1'

whwhwwhwwh avatar Jul 25 '24 08:07 whwhwwhwwh

Actually I think that block should be:

} else if (!useSharedMemory) {
   if (iOSSemVer && iOSSemVer.major <= 16 && iOSSemVer.minor < 4)
      sourceWasm = SorterWasmNonShared;
   }
}

It seems like we should only use the non-shared option for web assembly modules when it's iOS < 16.4. I honestly don't know how the non-shared web assembly modules work at all because when I instantiate the web assembly module here: https://github.com/mkkellogg/GaussianSplats3D/blob/cc61292dd57799b197f3330a9ceb32fa34b4de94/src/worker/SortWorker.js#L155 I always specify shared memory. This is only shared between the web assembly module and the web worker; it's different that the shared memory between the web worker and the viewer.

mkkellogg avatar Jul 25 '24 15:07 mkkellogg

Just circling back to this one -- I've made some slight changes to the WASM module selection for iOS in the memory-optimizations branch. Would you be able to try that out to see if it works for you?

mkkellogg avatar Sep 04 '24 04:09 mkkellogg

Going to close this for now, please let me know if you need more assistance with this one

mkkellogg avatar Sep 23 '24 21:09 mkkellogg