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

sokol_app.h: Resize callback is never unregistered giving javascript errors #983

Closed
edubart opened this issue Jan 28, 2024 · 3 comments
Closed

Comments

@edubart
Copy link
Contributor

edubart commented Jan 28, 2024

Problem

In HTML5 platform with html5_canvas_resize set to false, after sapp_run() finishes and you resize the Window, you will get lots of javascript errors, such as:

DOMException: Document.querySelector: '' is not a valid selector
    findEventTarget http://localhost:6931/test.js:1
    _emscripten_get_element_css_size http://localhost:6931/test.js:1
    uiEventHandlerFunc http://localhost:6931/test.js:1
    jsEventHandler http://localhost:6931/test.js:1
    registerOrRemoveHandler http://localhost:6931/test.js:1
    registerUiEventCallback http://localhost:6931/test.js:1
    _emscripten_set_resize_callback_on_thread http://localhost:6931/test.js:1
    _rivemu_start_record http://localhost:6931/test.js:1
    ccall http://localhost:6931/test.js:1
    rivemuRecord http://localhost:6931/test.html:155
    rivemuRecord http://localhost:6931/test.html:144
    rivemuInsertCartridge http://localhost:6931/test.html:136
    onclick http://localhost:6931/test.html:1
lockdown-install.js:1:94626

Solution

Adding this line to _sapp_emsc_unregister_eventhandlers fixes the issue:

emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, 0, true, 0);

Use case

I am trying to make a HTML page where you can test multiple sokol apps, each one app calls sapp_run() and take ownership of the canvas and at some point the app ends. To have this work properly sapp_run() should always cleanup nicely so the next sokol_run() can run.

@floooh
Copy link
Owner

floooh commented Jan 28, 2024

Yep makes sense. Do you want to provide a PR?

@edubart
Copy link
Contributor Author

edubart commented Jan 28, 2024

Just made the PR #984 to fix that.

@floooh
Copy link
Owner

floooh commented Jan 28, 2024

Thanks! PR has been merged. I'll write a small blurb in the changelog too.

@floooh floooh closed this as completed Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants