dictation-toolbox / Caster

Dragonfly-Based Voice Programming and Accessibility Toolkit
https://dictation-toolbox.github.io/Caster/
Other
340 stars 121 forks source link

Enhanced grids linux + complete Sudoku #897

Closed kendonB closed 3 years ago

kendonB commented 3 years ago

Get grids working on linux + complete Sudoku

Description

Related Issue

None.

Motivation and Context

Mouse grids don't work on Linux.

How Has This Been Tested

Verbally trying out the commands for Douglas/Rainbow/Sudoku on my Ubuntu machine with two monitors. I also tried Rainbow on my Windows machine but someone else should test these still work on Windows.

Types of changes

Checklist

Maintainer/Reviewer Checklist

kendonB commented 3 years ago

@BenChand would you like to review this PR?

LexiconCode commented 3 years ago

Great minds think alike as I was looking at making the grid use function context last night. Something like the following but I think achieves the same goal without another dependency. I will post my Branch when it's ready and see what you think. Based on that we will move forward.

LexiconCode commented 3 years ago

There's an issue with how we're checking for the grid with BenChand addition. Basically it doesn't account for canceling the grid and it thinks grid system still running.

LexiconCode commented 3 years ago

self.hide() is meant to hide the overlay causing the window to become invisible until it's needed to be redrawn. Worst case scenario if this is needed on others OS we can put it behind the platform check.

LexiconCode commented 3 years ago

Would it make sense to separate the PR into 2 separate pull requests. One for Function Context and one for Linux support?

See what you think of the following for function context as it doesn't rely on psutil although it doesn't have your Linux fixes nor standardization assumes a grid system. https://github.com/dictation-toolbox/Caster/compare/master...LexiconCode:mouse_grid?

kendonB commented 3 years ago

I support your approach for detecting the grid context. It is cleaner than mine. I haven't noticed any performance difference between the two branches though - on ubuntu.

@LexiconCode If you make a PR let's merge yours first then I'll add my linux and sudoku changes.

kendonB commented 3 years ago

need to remember to add to requirements as well so the checks pass

LexiconCode commented 3 years ago

What's your rationale for pyscreenshot over PIL ImageGrab? They both should be cross-platform however I definitely could be missing something.

LexiconCode commented 3 years ago
* Remove `self.hide()` in Rainbow. This was causing the grid to draw on the wrong monitor in a two monitor setup. **This is what I'm most uncertain about. I don't know what function it served previously. We'll need to test on multiple systems.**

With Windows the grid appears blank until the rainbow grid is rendered. self.hide() hides the blank grid until it's rendered and then the grid appears. Resolved with the following:

        if sys.platform == "win32": 
            self.hide() # On Linux: Wrong monitor when grid draw
kendonB commented 3 years ago

What's your rationale for pyscreenshot over PIL ImageGrab? They both should be cross-platform however I definitely could be missing something.

ImageGrab is not cross platform: https://stackoverflow.com/a/43636492/2726309

LexiconCode commented 3 years ago

@kendonB The reason I asked is because the docs make reference to Mac and Linux ImageGrab. It was something I only noticed when simplifying the legion grid. However we'll go with pyscreenshot. And what version of Python are you using?

kendonB commented 3 years ago

Hmm the stackoverflow may be outdated. Though I definitely only would have switched if I was getting an error with PIL. I will have another look later

kendonB commented 3 years ago

@LexiconCode my pillow was too old - I have reverted that bit and updated reqs to be pillow>=8.0.0

kendonB commented 3 years ago

recommend squash and merge for this one as well :)

LexiconCode commented 3 years ago

@kendonB

I need to add the conditional check for windows 'self.hide()'. This PR is looking great thank you!

It'll be a day or so before I can do the final check also testing on Python 2.7.

LexiconCode commented 3 years ago

Depending on how you tested I bet the issue isn't hide but unhide.

These different ways to set focus came about because Windows is notoriously unreliable about ensuring focus of the window. I would be curious if you commented out derivatives of focus and see if the behavior is the same. I thought about taking this out completely as we no longer rely on setting focus.

    def unhide(self):
        ''''''
        self.deiconify()
        self.lift()
        time.sleep(0.1)
        self.focus_force()
        self.focus_set()
        self.focus()

otherwise there's an issue with self.lift() and self.withdraw() and the platform check would be necessary.

kendonB commented 3 years ago

doing this

    def unhide(self):
        ''''''
        self.deiconify()
        self.lift()
        time.sleep(0.1)
        #self.focus_force()
        #self.focus_set()
        #self.focus()

but leaving self.hide() in also incorrectly prints the rainbow grid on the first monitor.

But if we're not actually using hide or unhide for anything then let's just get rid of it here in this PR I say.

kendonB commented 3 years ago

Part of the function of hide/unhide on rainbow might be because it draws the background image inside the PIL window. I don't know why it's set up this way because the PIL image is transparent. So you see the actual background image as well as the old captured one. You see both if something changed

LexiconCode commented 3 years ago

Part of the function of hide/unhide on rainbow might be because it draws the background image inside the PIL window. I don't know why it's set up this way because the PIL image is transparent. So you see the actual background image as well as the old captured one. You see both if something changed

Interesting I think it needs to be investigated further. That might be a better PR in the future.

LexiconCode commented 3 years ago

doing this

    def unhide(self):
        ''''''
        self.deiconify()
        self.lift()
        time.sleep(0.1)
        #self.focus_force()
        #self.focus_set()
        #self.focus()

but leaving self.hide() in also incorrectly prints the rainbow grid on the first monitor.

But if we're not actually using hide or unhide for anything then let's just get rid of it here in this PR I say.

Yes I am seeing a difference on Windows. Basically a blank gray window appears the size of the monitor before the rest of the window is drawn. hide successfully hides that until it's done being drawn.

Not a big issue it's something we can do a platform check to execute on other platforms and not Linux. I can push that to the PR as I have it done and removed the focus bits as are not needed anymore.

LexiconCode commented 3 years ago

Tests out on Python 2.7.