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

Make screen.find() more convenient on high-DPI devices #211

Closed
wants to merge 1 commit into from

Conversation

dscho
Copy link

@dscho dscho commented Feb 15, 2021

When capturing a screen region in a recent Windows version on a high-DPI device, the captured image can have a much higher resolution than scale-unaware applications such as node.exe might think (compare also this PR).

For convenience, let's support scaling the "needle" image that nut.js is told to find.

For example, on my laptop, where the scale factor is 200%, I would capture a rectangular region, save it as a .png file, then tell
nut.js to find it thusly:

await screen.find('needle.png', 10000, { scaleNeedle: true })

When capturing a screen region in a recent Windows version on a high-DPI
device, the captured image can have a much higher resolution than
scale-unaware applications such as `node.exe` might think (compare also
nut-tree/libnut-core#52).

For convenience, let's support scaling the "needle" image that `nut.js`
is told to find.

For example, on my laptop, where the scale factor is 200%, I would
capture a rectangular region, save it as a `.png` file, then tell
`nut.js` to find it thusly:

	await screen.find('needle.png', 10000, { scaleNeedle: true })

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Author

dscho commented Feb 15, 2021

The CI/sonar job fails with "Set the SONAR_TOKEN env variable.". Maybe it should be skipped if the environment variable is not set (i.e. for PR builds)?

@s1hofmann
Copy link
Member

Hi @dscho 👋

Thanks for your contributions!
I’m currently right in the middle of a move, so it’ll take some time until I can review your PRs.

I‘ll get back to you once things have settled a bit.

Best regards

Simon

@dscho
Copy link
Author

dscho commented Feb 18, 2021

I’m currently right in the middle of a move, so it’ll take some time until I can review your PRs.

I‘ll get back to you once things have settled a bit.

Sounds good. All the best for your move.

@s1hofmann
Copy link
Member

Hi @dscho,

I finally found some time to review both #211 and nut-tree/libnut-core#52.

nut.js is already able to deal with high DPI displays, since this problem already occurred on macOS retina displays.
It determines the scale factor by comparing the actual image size against the reported screen size.
However, the problem at hand comes from how libnut captures the screen content on Windows.
A regular screen capture uses the reported screen size, thus it only captures e.g the upper left quarter of your screen due to the actual screen content being larger. So if copyMMBitmapFromDisplayInRect would report the actual image size, nut.js would be able to deal with high DPI displays right away.

When I first reviewed your PRs I've been a bit overwhelmed by their changes. Especially the runtime library loading and image resizing got me thinking about alternative ways to deal with high DPI screens on Windows. I'd love nut.js to receive the plain image data without previously modifying it. Receiving unmodified data simply removes an additional source for possible errors / failures when using it for image recognition.

My implementation is only concerning libnut, so no changes to nut.js are needed.
It determines a scale factor based on the actual screen bitmap compared to the reported screen size.
This way libnut reports the actual image size and nut.js is able to properly process it.

Unfortunately I'm only running a Windows VM for development, so if you want to test my approach on an actual machine, I'd love to hear your feedback.
I pushed my implementation at https://github.com/nut-tree/libnut/compare/feature/183/windows_high_dpi

@s1hofmann s1hofmann self-requested a review March 4, 2021 22:32
Copy link
Member

@s1hofmann s1hofmann left a comment

Choose a reason for hiding this comment

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

Given a way to solve this issue without applying changes to nut.js I'd love to discuss possible solutions

@dscho
Copy link
Author

dscho commented Mar 13, 2021

Unfortunately I'm only running a Windows VM for development, so if you want to test my approach on an actual machine, I'd love to hear your feedback.
I pushed my implementation at https://github.com/nut-tree/libnut/compare/feature/183/windows_high_dpi

I tested this, and it doesn't work for me. My screen is 3240 x 2160 pixels, but even with your patch, libnut reports it to be 1620 x 1080.

@dscho
Copy link
Author

dscho commented Mar 13, 2021

Unfortunately I'm only running a Windows VM for development, so if you want to test my approach on an actual machine, I'd love to hear your feedback.
I pushed my implementation at https://github.com/nut-tree/libnut/compare/feature/183/windows_high_dpi

I tested this, and it doesn't work for me. My screen is 3240 x 2160 pixels, but even with your patch, libnut reports it to be 1620 x 1080.

Aargh, I looked at the wrong data. Your patch is actually really good!

Might I suggest just one thing? The following diff should make people with multi-screen setups happy:

diff --git a/src/win32/screengrab.c b/src/win32/screengrab.c
index 6842c6e..93c32a9 100644
--- a/src/win32/screengrab.c
+++ b/src/win32/screengrab.c
@@ -9,8 +9,8 @@ MMRect getScaledRect(MMRect input, HDC imageSource) {
 	HGDIOBJ hBitmap = GetCurrentObject(imageSource, OBJ_BITMAP);
 	GetObject(hBitmap, sizeof(BITMAP), &structBitmapHeader);
 
-	size_t desktopWidth = (size_t)GetSystemMetrics(SM_CXSCREEN);
-	size_t desktopHeight = (size_t)GetSystemMetrics(SM_CYSCREEN);
+	size_t desktopWidth = (size_t)GetSystemMetrics(SM_CXVIRTUALSCREEN);
+	size_t desktopHeight = (size_t)GetSystemMetrics(SM_CYVIRTUALSCREEN);
 
 	double scaleX = (double)(structBitmapHeader.bmWidth / desktopWidth);
 	double scaleY = (double)(structBitmapHeader.bmHeight / desktopHeight);

This will grab a full screenshot that encloses all attached screens as far as I understand (I cannot test this, though, I only have a single screen).

@dscho dscho closed this Mar 14, 2021
@dscho dscho deleted the respect-scale-factor-on-win10 branch March 14, 2021 00:51
@s1hofmann
Copy link
Member

Hi @dscho 👋

I used SM_CXSCREEN on purpose, since nut.js only supports single screen setups at the moment. I’m also limited to a single (virtual) screen so I’m unable to test this either.

Thanks for your feedback, I’m happy to have an actual Windows user test this. I’m limited to a Mac and several VMs. So if you’re interested you might want to join our Discord?

@dscho
Copy link
Author

dscho commented Mar 14, 2021

I used SM_CXSCREEN on purpose, since nut.js only supports single screen setups at the moment. I’m also limited to a single (virtual) screen so I’m unable to test this either.

Fair enough!

Thanks for your feedback, I’m happy to have an actual Windows user test this. I’m limited to a Mac and several VMs. So if you’re interested you might want to join our Discord?

I'd love to, but I fear that would be too much of a time commitment for me.

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.

None yet

2 participants