Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raster layer with a remote COG fails to load #4

Open
wonder-sk opened this issue Sep 26, 2023 · 4 comments
Open

Raster layer with a remote COG fails to load #4

wonder-sk opened this issue Sep 26, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@wonder-sk
Copy link
Member

When using a QGIS project that contains a cloud-optimized GeoTIFF (COG) on a remote server, it fails with a runtime error:

qgis-js.wasm:0x4792c6f Uncaught (in promise) RuntimeError: memory access out of bounds
    at strlen (qgis-js.wasm:0x4792c6f)
    at std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>>::basic_string[abi:v15006]<std::nullptr_t>(char const*) (qgis-js.wasm:0x932371)
    at CPLString::CPLString(char const*) (qgis-js.wasm:0x23aeac1)
    at cpl::VSICurlHandle::GetFileSizeOrHeaders(bool, bool) (qgis-js.wasm:0x242d5d3)
    at cpl::VSICurlHandle::GetFileSize(bool) (qgis-js.wasm:0x244222a)
    at cpl::VSICurlHandle::Exists(bool) (qgis-js.wasm:0x242f73e)
    at cpl::VSICurlFilesystemHandlerBase::Stat(char const*, stat*, int) (qgis-js.wasm:0x243cb26)
    at VSIStatExL (qgis-js.wasm:0x23e16c8)
    at GDALOpenInfo::GDALOpenInfo(char const*, int, char const* const*) (qgis-js.wasm:0x3d7e2c7)
    at GDALOpenEx (qgis-js.wasm:0x3db2ab9)

We are using our custom implementation of /vsicurl that uses emscripten's fetch API to do the requests.

It is strange that it crashes on some string construction. In the gdal sandbox this works fine (https://github.com/wonder-sk/wasm-gdal-sandbox), but the whole test case there runs in a worker thread because of the blocking networking calls.

@wonder-sk wonder-sk added the bug Something isn't working label Sep 26, 2023
@wonder-sk
Copy link
Member Author

Probably because of this call:

        const char *pszEffectiveURL = fetch->url;  // URL may change after redirect

where url is likely nullptr.... (when fetch is called on the main thread -> it immediately fails)

@wonder-sk
Copy link
Member Author

Okay so the issue is that in https://github.com/wonder-sk/wasm-gdal-sandbox I have used emscripten's synchronous fetch API - it worked because in the test repo I used PROXY_TO_PTHREAD emscripten option.

Using PROXY_TO_PTHREAD in qgis-js seems like a dead end, and Qt's WASM implementation does not seem to support that: at start, Qt emits warning that QApplication was not created in the main() thread, then there's a fatal error in QWasmIntegration constructor...

Another try was to avoid emscripten's synchronous fetch API, because it can't be used in the main thread. This would be done by using fetch() directly from EM_JS() in the vsicurl code, together with emscripten's asyncify. Unfortunately this is also a dead end for now, because emscripten does not support Asyncify with -fwasm-exceptions (that we use in qgis-js). See WebAssembly/binaryen#4470 and WebAssembly/binaryen#5475

Another try was to allow loading of GDAL provider in QgsProject in a worker thread, but that code uses QEventLoop in the main thread, which causes endless "QEventLoop:WaitForMoreEvents is not supported on the main thread without asyncify" logs in browser.

I think the proper solution would be that QgsProject is refactored to enable loading of QGIS projects in worker thread(s), with the main thread free to process events while loading. This would make things better also in desktop QGIS where the freeze during project load can be annoying. The refactoring may involve a substantial amount of work though...

@boardend
Copy link
Collaborator

I think PROXY_TO_PTHREAD and Asyncify should be avoided. In the long run, we probably want to have all processing to happen on a worker anyway (e.g. to prevent blocking the browser main thread, when opening a QGIS project).

We might be able to do that, if we span a main worker thread to load and interact with the QGIS project, and trampoline all API calls to that thread from the browsers main thread. (see https://emscripten.org/docs/api_reference/proxying.h.html)

The GDAL patch can then just fetch synchronously and we don't have to change anything in QGIS. This was working with the "osgearthwasm" prototype. But with the cost of breaking most of the stuff from the Qt WebAssembly platform plugin (which we are not relaying on at the moment).

@wonder-sk
Copy link
Member Author

In the long run, we probably want to have all processing to happen on a worker anyway (e.g. to prevent blocking the browser main thread, when opening a QGIS project).

Indeed!

We might be able to do that, if we span a main worker thread to load and interact with the QGIS project, and trampoline all API calls to that thread from the browsers main thread. (see https://emscripten.org/docs/api_reference/proxying.h.html)

My worry with that kind of approach is that it would add yet more complexity to the code, and it will be harder to understand all the juggling between the threads. My preference would be to fix the QGIS code, so that stuff that should be run in a worker thread also runs in desktop QGIS in a worker thread - maybe involving more work, but also more future-proof, keeping qgis-js wrapper smaller and improving UX for desktop QGIS...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants