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

GlfwApplication: Workaround for DPI scaling #388

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

costashatz
Copy link
Contributor

This PR offers a workaround for DPI scaling when monitor size is equal to zero in GlfwApplication..

@mosra mosra added this to the 2019.1c milestone Nov 8, 2019
@mosra mosra added this to TODO in Platforms via automation Nov 8, 2019
@mosra
Copy link
Owner

mosra commented Nov 8, 2019

Merged as 411e349, thank you!

@mosra mosra closed this Nov 8, 2019
Platforms automation moved this from TODO to Done Nov 8, 2019
@mosra
Copy link
Owner

mosra commented Jan 1, 2020

@costashatz looking at release notes of glfw 3.3.1, I see "zero monitor size" being fixed: https://www.glfw.org/changelog.html .. should we remove this workaround again? (I assume distros that already have 3.3 would get 3.3.1 soon, and those that are on 3.2 and older won't upgrade to 3.3.0 because that would make no sense.)

@mosra mosra reopened this Jan 1, 2020
Platforms automation moved this from Done to In progress Jan 1, 2020
@codecov-io
Copy link

codecov-io commented Jan 1, 2020

Codecov Report

Merging #388 into master will increase coverage by 1.67%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #388      +/-   ##
==========================================
+ Coverage    72.4%   74.07%   +1.67%     
==========================================
  Files         354      364      +10     
  Lines       18753    21099    +2346     
==========================================
+ Hits        13578    15630    +2052     
- Misses       5175     5469     +294
Impacted Files Coverage Δ
src/Magnum/GL/BufferTexture.h 20% <0%> (-5%) ⬇️
src/Magnum/SceneGraph/AbstractFeature.h 45.94% <0%> (-4.06%) ⬇️
src/Magnum/Shaders/Phong.cpp 75.4% <0%> (-3.67%) ⬇️
src/Magnum/Shaders/Flat.cpp 73.49% <0%> (-2.19%) ⬇️
src/Magnum/GL/RectangleTexture.h 5.88% <0%> (-1.81%) ⬇️
src/Magnum/GL/Context.h 90.69% <0%> (-0.97%) ⬇️
src/Magnum/GL/CubeMapTextureArray.h 1.13% <0%> (-0.4%) ⬇️
src/Magnum/Trade/LightData.cpp 100% <0%> (ø) ⬆️
src/Magnum/Animation/Interpolation.cpp 100% <0%> (ø) ⬆️
src/Magnum/Math/Quaternion.h 100% <0%> (ø) ⬆️
... and 94 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01e12ba...d62f1ba. Read the comment docs.

@costashatz
Copy link
Contributor Author

@costashatz looking at release notes of glfw 3.3.1, I see "zero monitor size" being fixed: https://www.glfw.org/changelog.html .. should we remove this workaround again? (I assume distros that already have 3.3 would get 3.3.1 soon, and those that are on 3.2 and older won't upgrade to 3.3.0 because that would make no sense.)

@mosra sorry for the late reply! I was on vacation..

I will check whether on my system this was fixed. Because this was a singularity container and maybe the version freezes to 3.3..

Does it hurt to keep it there? I mean performance-wise.

@mosra
Copy link
Owner

mosra commented Jan 13, 2020

No worries :)

Performance-wise it doesn't hurt, but it's a no-longer-needed cruft that enlarges the surface for potential bugs. If you can confirm this is no longer needed, I'll remove it, otherwise I'll at least add a note that this workaround is obsolete since 3.3.1.

@mosra mosra modified the milestones: 2020.0a, 2020.0b Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Platforms
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants