Skip to content

Fix bug for highdpi calculation in Viewer.cpp#2386

Merged
alecjacobson merged 1 commit intolibigl:mainfrom
DJAntivenom:bug_fix/viewer_high_dpi
Feb 18, 2025
Merged

Fix bug for highdpi calculation in Viewer.cpp#2386
alecjacobson merged 1 commit intolibigl:mainfrom
DJAntivenom:bug_fix/viewer_high_dpi

Conversation

@DJAntivenom
Copy link
Copy Markdown
Contributor

In Viewer.cpp the way the highdpi value was calculated could lead to it being set to 0 or inf for certain types of window managers. (Tiling window managers). This was caused by the WM trying to resize the window to a width and height of 0 by 0, or by having the logical width/height of the window be smaller than the pyhsical one. This would later cause a glfw call to be made with inf as an argument, which causes an assertion to be raised in debug mode.

Fixes (no issue exists).

  1. I added a check in Viewer::launch_init(...) method to check that we don't divide by 0.
  2. I fixed a problem where two ints would be used for a division, which would cause the value to be 0 instead of $\in [0,1]$.

Checklist

  • All changes meet libigl style-guidelines.
  • Adds new .cpp file.
  • Adds corresponding unit test.
  • This is a minor change.

The way the highdpi value was calculated could lead to it being set to 0 or inf for certain types of window managers.
(Tiling window managers). This was caused by it trying to resize the window to a width and height of 0x0, or by having
the logical width/height of the window be smaller than the pyhsical one. This would cause the `highdpi` variable to be
set to 0, which would later cause a glfw call to be made with `inf` as an argument.
@DJAntivenom
Copy link
Copy Markdown
Contributor Author

@alecjacobson Hi there, sorry for bothering. Do you think you could have a look at this?

@alecjacobson alecjacobson merged commit ba69acc into libigl:main Feb 18, 2025
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