Skip to content

Conversation

@doanamo
Copy link
Contributor

@doanamo doanamo commented May 14, 2020

Do not write output values when optional output pointer is null for calls of following functions:

  • glfwGetFramebufferSize (width/height)
  • glfwGetWindowSize (width/height)
  • glfwGetWindowPos (x/y)

This is in pair with native GLFW 3.2 implementation and documentation for these functions.

Example crash callstack and my code that ran into it:

RuntimeError: abort(segmentation fault storing 4 bytes to address 0) at jsStackTrace@http://localhost:8000/Example.js:2219:17
stackTrace@http://localhost:8000/Example.js:2236:16
abort@http://localhost:8000/Example.js:1995:44
SAFE_HEAP_STORE@http://localhost:8000/Example.js:931:23
setValue@http://localhost:8000/Example.js:875:34
_glfwGetFramebufferSize@http://localhost:8000/Example.js:9572:17
System::Window::GetWidth() const@http://localhost:8000/Example.js line 2133 > WebAssembly.instantiate:wasm-function[12079]:0x1c19c7
int Window::GetWidth() const
{
    int width = 0;
    glfwGetFramebufferSize(m_handle, &width, nullptr);
    return width;
}

int Window::GetHeight() const
{
    int height = 0;
    glfwGetFramebufferSize(m_handle, nullptr, &height);
    return height;
}

Thank you!

@welcome
Copy link

welcome bot commented May 14, 2020

Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good, just some whitespace changes needed before landing.


setValue(x, wx, 'i32');
setValue(y, wy, 'i32');
if(x) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after if please.

…ome functions

Do not write values when optional output pointer is null for following functions:
- glfwGetFramebufferSize (width/height)
- glfwGetWindowSize (width/height)
- glfwGetWindowPos (x/y)
@doanamo
Copy link
Contributor Author

doanamo commented May 14, 2020

Ops! Corrected and updated. :)

@kripken
Copy link
Member

kripken commented May 15, 2020

Perfect, thanks @gunstarpl !

@kripken kripken merged commit de021c8 into emscripten-core:master May 15, 2020
@doanamo doanamo deleted the glfw-crash-fixes branch May 15, 2020 00:25
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

Successfully merging this pull request may close these issues.

2 participants