-
Notifications
You must be signed in to change notification settings - Fork 7
mouseMoved
bugfix
#12
base: master
Are you sure you want to change the base?
mouseMoved
bugfix
#12
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like your idea to move shaders to separate files.
however, your refactored files are still, strictly speaking, C++ code.
for this reason i think it would not be considered best practice to have C++ code in .vert/.frag files.
the best solution would be to do this during the build process.
this problem has already been solved in libxd, check https://github.com/bernhardfritz/libxd/blob/master/CMakeLists.txt#L63
auto _fullscreenchangeCallback = [](int eventType, const EmscriptenFullscreenChangeEvent *fullscreenChangeEvent, void *userData) { | ||
((BaseApp*) userData)->fullscreenchangeCallback(fullscreenChangeEvent->isFullscreen); | ||
return 1; | ||
}; | ||
emscripten_set_fullscreenchange_callback("#document", this, true, _fullscreenchangeCallback); | ||
#endif | ||
#else | ||
auto _framebufferSizeCallback = [](GLFWwindow* window, int framebufferWidth, int framebufferHeight) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not sure if this callback should be native only. in case of web, resizing of canvas is also a scenario to consider.
src/piksel/baseapp.cpp
Outdated
static int oldFramebufferWidth = width; | ||
static int oldFramebufferHeight = height; | ||
static bool retinaDisplay = false; | ||
if (framebufferWidth == 2*oldFramebufferWidth and framebufferHeight == 2*oldFramebufferHeight) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not happy about this hack.
first problem is, it is not macOS specific, this can happen on every OS that supports content scaling.
second problem is, 2 times scaling is not the only available scaling.
i think it would be better to look for a more universal solution, like https://www.glfw.org/docs/latest/window_guide.html#window_scale
i did not try it myself but glfwSetWindowContentScaleCallback
looks very promising.
as to the main point of the PR, i'll experiment with |
Main changes in
updateFramebufferSize
to calculatedisplayPixelRatio
and re-use it when not compiling with Emscripten to report a correct mouse position (when compiling for the web theupdateFramebufferSize
isn't called because the canvas does not get resized). There is a bit of a hack ( in the#if defined(__APPLE__) && !defined(__EMSCRIPTEN__)
block) which attempts to detect when the window is dragged between a normal and a retina display (or back), since that was a problem for me.I believe this somewhat fixes #9.
I've also pulled out the shader code into separate files to make it easier edit and actually use code highlighting on. Absolutely no changes in the shader code.