frakman1 / lifx-lan-gui

LIFX LAN GUI Controller using appJar
MIT License
21 stars 5 forks source link

Screenshot window for selecting desktop area is too large #1

Closed mclarkk closed 6 years ago

mclarkk commented 6 years ago

Thanks for making this great GUI! I especially like the "Follow desktop" feature, but when the screenshot window pops up on my laptop, it's enormous -- way bigger than my screen. The window is also not resizable, so I can't fit it all on the screen. This means I can only select a part of the screen to follow.

I'm running lifx-lan-gui on macOS High Sierra 10.13.3. I also regularly plug in to a 34" monitor in clamshell mode. It's probably irrelevant, but switching back and forth from the monitor seems to confuse the laptop's display sometimes, so maybe it's part of it?

frakman1 commented 6 years ago

My pleasure. Thank you for bringing this issue up. I intentionally made the screenshot window full-screen to make it look like the screen you were viewing:

        cv2.namedWindow("Screenshot", cv2.WINDOW_FULLSCREEN) 

However, I've never tried it with a second monitor but I am pretty sure that would explain the enormous size. For now, try using it without an external monitor. I will look into making the window resizeable but I am not sure if that will impact the selection rectangle result.

frakman1 commented 6 years ago

I don't have an external monitor for my Mac to test with at the moment. Can you please try using this code instead and see if that works better for you?

        cv2.namedWindow("Screenshot",cv2.WINDOW_AUTOSIZE) 

Thank you

mclarkk commented 6 years ago

That didn't work either!

I think what I said about the external monitor was confusing...it's probably best if we forget I ever said anything about external monitors. (I haven't tried to use the GUI while plugged into an external monitor at all. I just thought that maybe since the laptop had been plugged into an external monitor previously it might still retain some settings somewhere. If that was the case though I would expect a reboot to clear it, and it didn't.)

mclarkk commented 6 years ago

This works!

        # Convert RGB to BGR
        im = open_cv_image[:, :, ::-1].copy()
        screen_width = app.winfo_screenwidth()
        screen_height = app.winfo_screenheight()
        imS = cv2.resize(im, (screen_width, screen_height))
        cv2.namedWindow("Screenshot", cv2.WINDOW_AUTOSIZE)
        cv2.imshow("Screenshot", imS)
        cv2.moveWindow("Screenshot", 0, 0)
        r = cv2.selectROI("Screenshot", imS, False)
mclarkk commented 6 years ago

Actually, even that didn't quite show the whole picture. Because there is a margin at the top of the image within the frame, some of the bottom is cut off, though the width is perfect. I ended up shrinking it further:

imS = cv2.resize(im, (int(screen_width*0.9), int(screen_height*0.9)))

It's less pretty, but I can see the whole thing this way.

frakman1 commented 6 years ago

Thank you. I love it when developers work to improve my code! Looks like it's getting there.

My only worry is that the selected rectangle will be off relative to the real screen since it was selected on a window with a resized image. If I can recreate your scenario, I will test the corner cases and boundaries to make sure.

Regarding external monitor. hmm, I'm testing on a Macbook Air and my screenshot fits nicely in the screen. Maybe the higher Retina resolution is messing things up. I have a Macbook Pro in the office (with an external monitor) I will try later.

mclarkk commented 6 years ago

I have not used lifx-lan-gui with an external monitor. I have only used it on the laptop alone. Sorry that was confusing earlier!

frakman1 commented 6 years ago

Ideally, we would use:

        cv2.namedWindow("Screenshot",cv2.WINDOW_NORMAL) 

This allegedly allows the user to resize the window. However, when I tried it, I get a fatal crash that takes down python3 too!

I opened an issue with appJar to track it.

However, it works when I try it on Windows

mclarkk commented 6 years ago

Yeah, trying cv2.WINDOW_NORMAL earlier crashed the program on my Macbook as well.

frakman1 commented 6 years ago

Until the cv2.WINDOW_NORMAL issue is resolved, I think I will make a special if os=Mac case with your recommended code above and use cv2.WINDOW_NORMAL for Windows

frakman1 commented 6 years ago

Addressed in commit: https://github.com/frakman1/lifx-lan-gui/commit/561dc4957eaa4eb800fb3e7511869c21eeff70b5

frakman1 commented 6 years ago

Ignore the white-space in the commit diffs. The crux of it is here:

if (myos == 'Linux') or (myos == 'Darwin'):
    screen_width = app.winfo_screenwidth()
    screen_height = app.winfo_screenheight()
    im = cv2.resize(im, (int(screen_width*0.9), int(screen_height*0.9)))
    cv2.namedWindow("Screenshot", cv2.WINDOW_AUTOSIZE)
    cv2.imshow("Screenshot", im)
elif (myos == 'Windows'):
    cv2.namedWindow("Screenshot", cv2.WINDOW_NORMAL)