rticau / ScreenCapLibrary

Robot Framework test library for capturing screenshots and video recording.
Apache License 2.0
40 stars 15 forks source link

snapshot issue with "Take Partial Screenshot" keywords #49

Closed davesliu closed 4 years ago

davesliu commented 4 years ago

Hello.

I use the a monitor with resolution 1200*1920 and use the "take partial screenshot" keywords with set the top left coordinate to (200,300). Take Partial Screenshot name=test2.png format=png left=200 top=300 quality=100

It will generate below error message: FAIL : SystemError: Top and left parameters must be lower than screen resolution.

If I set the left=0 top=0, then I found that the snapshot size is 700*200 (700 is width and 200 is height)

Could anyone give some suggestion what's wrong with my script or if it's a bug of this library?

BTW, I use the take screenshot keyword and it can take the full screen size 1200*1920 correctly

davesliu commented 4 years ago

I debug the code, it seems that it crashed in the crop function Image. https://github.com/mihaiparvu/ScreenCapLibrary/blob/967026984fc82c8cb1a66637a4343015290d8f0b/src/ScreenCapLibrary/client.py#L214

davesliu commented 4 years ago

I can use work around such as pyautogui to get the partial screenshot but still want to know if this issue can be fixed or not. Thanks.

cristii006 commented 4 years ago

Hi @davesliu ,

First of all thanks for your time spent in other to analyze the problem it means a lot. I was looking into the problem you reported and I think there is a misleading documentation for the Take Partial Screenshot. How does this keyword work now is that you have top and left parameters that specifies from where your screenshot will be taken with respect to top respectively left sides of your screen. Now the tricky part is when using the other two parameters i.e. width and height.

What they do is they specify the total width/height you want to capture. So when you use top=0 and left=0 you will obtain a screenshot of 700X300(700 and 300 are the default values for width and height). Now when you set your top to be 300 what the program does it will take a screenshot that will start from 300th pixel but since your total height is also 300 your actual screenshot would be 300-300=0 and that generates an error.

I think height and width should be perceived as lower bound of your screenshot respectively rightmost bound. What could be done on order to avoid confusion would be either rename the width and height parameters and update the documentation or we can change the mechanism behind such that width and height are computed such that they will represent the final dimensions of the screenshot.

Best regards, Cristi

davesliu commented 4 years ago

@cristii006 Thanks for your clarification. It seems that the width and height are the coordinate of the bottom-right corner based on your comments. I will add the offset for snapshot calculation.

BTW, in below code, it will snapshot all screen and then use crop function to get the partial snapshot. I think it maybe a little low efficient to do full screen snapshot and then crop. https://github.com/mihaiparvu/ScreenCapLibrary/blob/967026984fc82c8cb1a66637a4343015290d8f0b/src/ScreenCapLibrary/client.py#L211

Is it possible to do partial snapshot directly without snapshot full screen first? (pyautogui do it with this way)

davesliu commented 4 years ago

@cristii006 I looked at the manual of crop function: Syntax: PIL.Image.crop(box = None) Parameters: box – a 4-tuple defining the left, upper, right, and lower pixel coordinate.

So in line https://github.com/mihaiparvu/ScreenCapLibrary/blob/967026984fc82c8cb1a66637a4343015290d8f0b/src/ScreenCapLibrary/client.py#L213

We can fix the issue by adding two lines before it and update this line right = left + width low = top + height box = (left, top, right, low)

cristii006 commented 4 years ago

Decided to go with modifying the cropping mechanism so it does what it is supposed to do and also keep a constant behavior between mss and PyGtk.