Do not stop blocking mouse events during canvas Enter event
ClosedPublic

Authored by abrahams on Feb 9 2016, 9:03 PM.

Details

Summary

Remove two lines of code that were difficult to justify. Doesn't make
an impact on my machine but should hopefully prevent full-pressure blobs.

Diff Detail

Repository
R37 Krita
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
abrahams updated this revision to Diff 2260.Feb 9 2016, 9:03 PM
abrahams retitled this revision from to Do not stop blocking mouse events during canvas Enter event.
abrahams updated this object.
abrahams edited the test plan for this revision. (Show Details)
abrahams added a reviewer: Krita.
abrahams added a subscriber: kamathraghavendra.

@kamathraghavendra Could you review this patch with arc patch D930 or just by commenting out those two lines of code and seeing if it fixes your problem?

@abrahams I tested this and unfortunately the issue persists. I also did a clean build but it didn't help.

abrahams added a comment.EditedFeb 11 2016, 6:45 AM

@abrahams I tested this and unfortunately the issue persists. I also did a clean build but it didn't help.

This is very confusing, I'm looking through the source to find another way that could generate the log you provided but I can't figure it out. It really seems like there shouldn't be any more unblocked MousePress events. Could you share a log of the blobs when you build with the patch?

@abrahams I commented the lines in the file and built it, here is the tablet events log

rempt added a subscriber: rempt.Mar 4 2016, 11:56 AM

Hm, even with git blame it's hard to figure out why these lines are here. But I'm not sure I even understand the input manager at this point.

abrahams added a comment.EditedMar 6 2016, 11:21 PM
In D930#20248, @rempt wrote:

Hm, even with git blame it's hard to figure out why these lines are here. But I'm not sure I even understand the input manager at this point.

Yeah. I don't think it's quite possible to understand fully because it's not fully specified. Before I did the Krita 3 rewrite, it relied on logic that threw away too many events. This prevented a wide variety of splotches and glitches, but at the cost of some smoothness. To write an input manager that guarantees it will accept every single tablet event, you have to handle many other complexities. Basically we need to handle a smattering of cross-platform, cross-hardware inconsistencies about events arriving in the wrong order, or with delays, or not pairing TabletEvents/MouseEvents evenly, without throwing any events away.

The logic needs to be refactored into a separate state machine of some sort that can do all the filtering in one place. The classes EventEater, CanvasSwitcher and all of this ignore_cursor_events stuff should be collected into one place with clear explanation of the various cases we're trying to handle.

(BTW the problem that remains for @kamathraghavendra is due to the call to stop_ignore_cursor_events() after QEvent::Leave. But I believe this call is more important than the one for QEvent::Enter.)

rempt accepted this revision.Mar 7 2016, 7:14 AM
rempt added a reviewer: rempt.

Yes, let's go with this now and see what happens...

This revision is now accepted and ready to land.Mar 7 2016, 7:14 AM

Unfortunately this patch doesn't solve @kamathraghavendra's problem. It seems like of a "no-op" as far as the inputmanager logic is concerned, but it could also randomly break something. I'd rather land something that fixes the problem, or perhaps make a bigger change...

Hi @kamathraghavendra - could you comment out lines 114 and 117 in kis_input_manager_p.cpp? #if defined(Q_OS_WIN)

Hi @abrahams ,

I commented the line 114 and 117 and did a clean build , but the issue persists :(
I wanted to ask is there a need to comment line 116 also -

peckish = true;

let me know if you want anything else.

thank you

Just wanted to update that with qt 5.6 I am not experiencing any issues related to tablet now, not the popup or button problem. I am going to test it thoroughly though.

abrahams closed this revision.Apr 4 2016, 4:33 PM

Luckily this spooky stuff seems to have been resolved in Qt 5.6.