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

use GetWindowScaleDPI to calculate size for rlReadScreenPixels in sceenshot/recording #2446

Merged

Conversation

gulrak
Copy link
Contributor

@gulrak gulrak commented Apr 27, 2022

On macOS when running on a display with retina resolution or on an external 4k screen, the TakeScreenshot function triggered by F12 and the gif recording triggered by Ctrl-F12 ignore the scaling so they only grab the lower left quarter of the window.

This change uses the factors from GetWindowScaleDPI() to scale width and height to allow the full window content to be grabbed even if scaling is active.

Not sure if there would be need for a check of FLAG_WINDOW_HIGHDPI, from my understanding, the scaling would be needed there to, for other cases if should be a scaling by 1, and the overhead of GetWindowScaleDPI() seems small compared to the gif or png encoding that happens afterwards.

@raysan5
Copy link
Owner

raysan5 commented May 1, 2022

@gulrak I'm afraid there have been many attempts to implement this same solution and everytime it broke some platform in some specific use case. I prefer to let the user manage it manually as required, that's the reason to expose GetWindowScaleDPI() for the user instead of applying it directly by raylib.

Still, I'm not closing the PR, I want to take a closer look to it.

@gulrak
Copy link
Contributor Author

gulrak commented May 1, 2022

Yeah, I understand the problem, the easy way to reduce that risk would be to tie it to the Plattform, still I'm planning to test it on Windows with and without high dpi flag and scaling and the same on Linux, just need to get some time for it, so no hurry.

@raysan5 raysan5 merged commit 789e504 into raysan5:master May 20, 2022
@raysan5
Copy link
Owner

raysan5 commented May 20, 2022

@gulrak After cheking this improvement more carefully I decided to merge it.

@D-T-666
Copy link

D-T-666 commented May 23, 2022

yay!

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.

3 participants