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

Screen reconfiguration not handled correctly #79

Closed
andreasbuhr opened this issue Nov 19, 2018 · 7 comments · Fixed by #80
Closed

Screen reconfiguration not handled correctly #79

andreasbuhr opened this issue Nov 19, 2018 · 7 comments · Fixed by #80

Comments

@andreasbuhr
Copy link
Contributor

General informations:

  • OS name: Linux Mint
  • OS version: xenial
  • OS architecture: 64 bit
  • Resolutions:
    • Monitor 1: 1920x1080
    • Monitor 2: 1920x1080
    • Monitor 3: 1920x1080
  • Python version: 2.7.12
  • MSS version: 3.3.1

Description of the warning/error

  1. start a python script while only one monitor connected
  2. take a screenshot
  3. connect more monitors
  4. take a screenshot of monitor "-1"
  5. only the monitor attached when taking the first screenshot is screenshoted.

Full message

no message

Other details

The reason is that the "_monitors" is a class varable of MSSBase

_monitors = [] # type: List[Dict[str, int]]

and it is only initialized once per script run:

if not self._monitors:

One possible solution is to turn "_monitors" into a member variable.

@BoboTiG
Copy link
Owner

BoboTiG commented Nov 19, 2018

Hello,

Thanks for the report and analysis!

Turning _monitors to a member variable would slow the screen capture by a too important factor. An idea would be to regularly set sct._monitor = [] to force an update. But this will need to be done outside MSS, in your code.

If we could set a hook to know when a monitor is added or removed, it would be the best way: efficient and easy. I do not know of such API but some research will have to be done.

Do you have time to investigate? The process can be only related to one OS, others will eventually come later.

@andreasbuhr
Copy link
Contributor Author

What about having the monitor detection in the MSS constructor? That way it is not done in every screenshot, but only when the MSS object is created? That way users could decide how often monitor detection is done, so to have a script taking screenshots every minute and doing monitor detection every time, one would have:

while True:
    time.sleep(60)
    with mss() as sct:
        sct.shot()

and for low overhead one would use:

with mss() as sct:
    while True:
        time.sleep(1)
        sct.shot()

Would that be a solution?

@BoboTiG
Copy link
Owner

BoboTiG commented Nov 20, 2018

Hm this is already the case :)
The first snippet will detect new monitors as expected because when the context manager ends, it will freed resources.

@andreasbuhr
Copy link
Contributor Author

I'm sorry, but it is not the case. You are mistaken.

The first snippet will not detect new monitors as expected, because _monitors is a class variable. It is a member of the class, not a member of the object. So _monitors will persist until the end of the interpreter run, unless it is deleted explicitly. MSSBase.__exit__ does not delete it:

def __exit__(self, *_):

Inserting a _monitors = [] into MSSBase.__exit__ would solve this problem.

If you like I could prepare a pull request.

@BoboTiG
Copy link
Owner

BoboTiG commented Nov 20, 2018

Good catch, I stand corrected.

I would be very happy to merge your PR :)
Could you add a simple test to validate the fix also?

@BoboTiG
Copy link
Owner

BoboTiG commented Nov 20, 2018

Release 3.3.2 is published with your patch :)

@andreasbuhr
Copy link
Contributor Author

amazing!!1!! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants