Hi, Michael!

In D256#4556, @abrahams wrote:

This seems good to me if you also comment out the windows event handler. (Right now I get bizarre crashes when we handle our own events... a stack trace offers nothing but qt internals.)

Did I understand it right that

1. you tried the patch and with the windows workaround not commented out you get a crash?
2. with the windows part commented out you get the effects you explained below?

One remaining problem: On Windows, mapping the tablet to the screen is not working correctly. The cursor's origin is getting mapped to (0,0) on my left monitor, bypassing the wacom settings which request the cursor be mapped to the middle monitor. However I believe that's a separate issue. I get the same behavior without applying this patch, but comment out the native event filter only.

This should probably be reported to Shawn Rutledge via IRC or Qt bugreports.

I have another question. Suppose Qt were to keep the promises of its API, sending tablet events with all the correct cursor information and scaling, and sending fake mouse events only if we reject the tablet event. If Qt plays nicely, can we play nicely too, and get rid of ~80% of KisInputManager, without tucking events into our pocket and saying "oops, I lost it, can I have another??"

Well, we will be able to get rid of KisTabletSupport* classes only. KisInputManager will still have to do quite a lot of work, like compression and working around some weird stuff of Qt. Right now, for example, Qt doesn't send any tablet events to the app when the cursor is hovering over the surface.

The main question now is:
Should we stick to Qt5's implementation of tablet events or fork it again? The fork is expensive, especially on Linux, where Qt now uses XInput2, but the effects might be quite good, especially on Linux, where we don't ship Qt with Krita package.

I think we should apply this patch and initially go with Qt's tablet support because then we can move on with the rest of the frameworks port, and start moving feature development like the LOD and animation branch to frameworks. Ideally, by the end of this month, after the next 2.9 release, I'd close the kdelibs4/2.9 version for feature development and allow only bug fixes. Then in September and October I'd want to really work on the port, reducing kf5 dependencies so there are no dbus, no kded5 using dependencies left.

If, by the time that stabilizes, it's clear that we need our own tablet code, we can re-enable and re-invent our tablet support hacks.

Hi Dmitry!

Did I understand it right that

1. you tried the patch and with the windows workaround not commented out you get a crash?
2. with the windows part commented out you get the effects you explained below?

I looked into the crash more closely. It was due Qt trying to handle tablet events while the screen resolution dialog was open, and commenting out the dialog fixed the crash. When entering Qt's tablet handler, we tried to look for a tablet device before one had been added. We just failed an assertion that the size of the device list was greater than zero, so I think on a release build this would technically be undefined, not a crash.

Another change that prevented crashes was altering nativeEventFilter to return true for Qt tablet events. The effect of returning true is to tell Qt that we handled the event on our own, masking Qt's internal tablet event handler. Is there a reason that handling all wintab events is undesirable? I posted the change as a diff here. D264

Unfortunately this change is incompatible with our own tablet handler, and the quality of drawing on Windows is diminished. I think we need to think harder about whether we should rely on the Qt tablet handler.

Well, we will be able to get rid of KisTabletSupport* classes only. KisInputManager will still have to do quite a lot of work, like compression and working around some weird stuff of Qt. Right now, for example, Qt doesn't send any tablet events to the app when the cursor is hovering over the surface.

Hmm. Getting rid of our custom handler may or may not be a good idea, depending on the answer to the following question: how much of this weird Qt stuff can we work around, and how much do we think we can convince them to change upstream?

KisTabletSupport* and KisInputManager are tied together by a contract: I will send events in this order, under these conditions. With our own handler, we can ensure these conditions are sane.

I don't really know much about compression or Qt weirdness in general. A major question I have though: with the Qt tablet handler, if we accept every tablet event, is it possible to guarantee we never receive fake mouseMove events? If that is the case, we can greatly simplify KisInputManager by no longer assuming we'll receive mouseMove events on the canvas. If we do that, then their decision not to distinguish between mouseMove and tabletMove events is not an issue, because we won't have to distinguish between the two either.

If Qt does have a sane event loop at the bottom of it all, it is in our advantage to use it. If Qt makes guarantees but they aren't fulfilled, we should hold them to it with bug reports. On the other hand, if Qt makes no promises about having reasonable behavior, we will have to use our own handlers either way.

This should probably be reported to Shawn Rutledge via IRC or Qt bugreports.

OK, will do.

One other thing I realized, after another look: Using this code and commenting out Windows event handler decreases the quality of my lines with my intuos 4, same thing happens on linux. I noticed that although QTabletEvent does not have a hiResGlobal in its constructor, but it does have hiResGlobalX() and hiResGlobalY() methods.

I think the decision should be discussed in T530.

I'm going to try and look at the input manager and tablet patches today, but I'm confused which ones I should apply...

In D256#5648, @rempt wrote:

I'm going to try and look at the input manager and tablet patches today, but I'm confused which ones I should apply...

Hi Boudewijn - the two solutions right now are this patch D256, and D270. Both are single patches intended to get tablet support working. D256 is a tighter change to KisInputManager internals, while D270 is a larger change that alters the interface of KisInputManager's client classes to simplify its internal design.

My opinion is that if the alterations applied in D270 are feasible, they should be done, because they involve less fighting against the Qt event stream, and more going with the flow. The internal benefit is to streamline the PrimaryAction/SecondaryAction API and the ShortcutMatcher. If the design of D270 is sound and the break with the Calligra libraries occurs (or some other sort of reorganization occurs such that KisTool no longer inherits from KoTool) this could eventually lead to separating Qt input events from tools altogether.

The other patch D264 is a windows-specific fix for the tablet handler. The point is to combine it with D256 on Windows, but it doesn't affect Linux.