[Development] possible API freeze exception: would like to introduce QQuickItem::setAcceptTouchEvents() in 5.9

[Robin Burchell]

Hi,

Let me start off by saying that I agree with you that this is a bad
asymmetry, both for the end user, and for our own internal purposes --
which appears to be your primary motivator. I'm happy to see this
addressed.

On the other hand, this is a rather large change in behavior (whereby as
I understand it, touch will stop working for custom items, and I know
that there are a lot of them out there that take touch events) that will
require code changes to adapt to. Not everyone can adapt their code --
some projects don't change after they release, some projects have other
priorities, etc. In my opinion, requiring this kind of a change with
only one minor release's notice is not something we should be doing
given our focus on backwards compatibility. I'd be interested in hearing
other opinions on list, so far I think I'm a tentative -1 to the idea.

You mention that this is required due to optimizing the way that event
delivery works internally in QQuickWindow -- have you considered
restoring the old behavior there (despite it not being ideal, I think
there's no disagreement there) possibly behind QT_MAJOR_VERSION guards,
and switch it over to requiring registration for Qt 6?

The reason that I ask is that I haven't ever seen what I would call a
significant portion of time being taken up in the transversal part of
event delivery -- it is of course non-zero, but the event handlers
themselves (e.g. an onClicked handler in a MouseArea) are way, way more
expensive which dwarf that cost in my experience...

Robin

-- 
  Robin Burchell
  robin at crimson.no

On Tue, May 9, 2017, at 06:43 PM, Shawn Rutledge wrote:
> This is of concern only to those who subclass QQuickItem in C++ and build
> custom items that handle touch events.
> 
> There is a pretty bad API asymmetry which it’s surprising that we didn’t
> notice enough to try to correct it earlier.  If you want to accept mouse
> events while any mouse button is clicked or held down or released, you
> call setAcceptedMouseButtons().  QQuickWindow won’t bother trying to
> deliver those events if you haven’t done that.  Likewise if you want to
> handle all mouse movement events, pressed or not, you must call 
> setAcceptHoverEvents(true).  Imagine a world in which these didn’t exist:
> QQuickWindow would need to deliver every mouse movement to every Item. 
> It would be unmanageable, performance would be unacceptable.
> 
> The docs say that QGraphicsItem::setAcceptTouchEvents(bool) has existed
> since Qt 4.6.  If you are handling touch events in a graphics view
> application, it’s required to call this.
> 
> Yet for some odd reason, QQuickItem::setAcceptTouchEvents(bool) was never
> added.
> 
> In some parts of the code, we make the assumption that if you have called
> setAcceptedMouseButtons(), you might want the touch events too.  In other
> parts, we don’t check… so in the simplest case, if you create a custom
> item that handles touch events and stick it into your scene, you might
> get the touch events with no further ado, if no other item prevents it.
> 
> It’s now become unmanageable to continue without adding this function. 
> QtQuick Controls 2 has an issue that if you have a Popup with a shadow
> Item, you can drag via touch within the thin shadow area around the
> popup, and the events will go through, and you’ll be able to drag the
> sidebar open for example, even though there’s a translucent Item in
> between which is supposed to both dim the rest of the application, and
> catch all the events, to prevent interacting with other controls while
> the Popup is open.  (Modal behavior.)
> 
> In 5.10 we hope to introduce PointerHandlers, a big improvement on
> MouseArea, PinchArea and MultiPointTouchArea.  This has been in the works
> for a couple of years now.  So on the wip/pointerhandler branch, we
> already have the requirement that setAcceptTouchEvents() must be called
> if a custom Item wants to accept touch events.  I thought maybe I could
> find a way to make it optional: to avoid changing behavior of existing
> custom items out in the wild, we would need it to default to true.  That
> is, assume that every Item might handle touch events, give every item a
> chance… because it was that way before.  Or at least, go on assuming that
> if it wants mouse events, maybe we should try to deliver touch events
> too.  But along with introducing PointerHandlers, we have tried to
> optimize event delivery and make it more deterministic: before we start
> delivery, we build up a vector of all items to which delivery will be
> attempted, in the right order (reverse paint order).  Then we build up a
> vector of pairs of items and their filtering parents (those which have
> called setFiltersChildMouseEvents - Flickable being the main example of
> that).  So when we are doing the delivery, we no longer need to go up the
> parent hierarchy for each and every item that we visit, looking again and
> again for filtering parents.  We no longer need a QSet just to keep track
> of items that we have visited, to avoid visiting them again.  BUT: if we
> cannot know which items accept touch events and which don’t, this doesn’t
> work, because we would have to include _every_ item which has a filtering
> parent in that list of pairs, whether it would accept the touch event or
> not.  And the result of that is that we will call childMouseEventFilter()
> too early in some cases, with the wrong item.  "Hey Flickable, you’ve got
> a Rectangle which is your child, I’m about to send it a TouchEvent, do
> you want to intercept it?”  Isn’t that stupid?  Everybody knows Rectangle
> doesn’t do anything with pointing-device events.  But if Flickable
> intercepts the event anyway, that means the MouseArea inside the
> Rectangle won’t get exactly the same chance it had before (even though
> Flickable is passive until the drag threshold is exceeded).  Of course
> Rectangle could call setAcceptTouchEvents(false) explicitly, but that
> doesn’t take care of the custom items.  Various Qt modules have custom
> items too; so we don’t want to start getting bugs about those misbehaving
> whenever they are used inside Flickable, for example.
> 
> So to get the event delivery optimized as much as possible, it’s got to
> be mandatory at some point to _subscribe_ in some way for touch events if
> you need them.  We can’t just assume, and spam every item with all
> events.  Assuming that wanting mouse events = wanting touch events is
> wrong too.  (It only made sense in a world in which all touch event
> delivery was done by synthesizing artificial mouse events from
> touchpoints.  And that bad decision, the very worst architectural
> decision in all of QtQuick, has caused several years of frustration for
> me.)  But I’ve been trying really hard to avoid changing any existing
> behavior, to avoid breaking any existing tests, which is why we didn’t
> come to this impasse sooner, I suppose.
> 
> So I simply want to add the functions setAcceptTouchEvents(bool) and bool
> acceptTouchEvents() right now, in 5.9.0.  Behavior won’t change yet;
> calling them will not be mandatory.  But in 5.10, it might be mandatory.
> 
> By the way, when we introduce PointerHandlers, the need to create custom
> Items to handle touch events will be mostly obsolete anyway.  (Still
> supported of course, for now.)  A PointerHandler is not an Item: it’s a
> lighter weight QObject which exists just to handle events.  Like a
> behavioral facet of your visual item, rather than needing to be a
> full-fledged (heavy) item like MouseArea is.  You will be able to have
> multiple PointerHandlers per Item.  There will be public C++ API for
> them.  So even in complex use cases, you should be able to create custom
> PointerHandlers rather than custom Items, if you don’t need to customize
> the visual appearance.  You won’t need to call setAcceptTouchEvents() on
> the parent Item in order to enable the PointerHandlers, either;
> QQuickWindow is already aware of them, and offers them events at the
> appropriate time.
> 
> I think that since mouse emulation has been enough for most use cases for
> this long, there probably aren’t that many custom items out there
> handling touch events directly; and for those that do, maybe it’s not too
> much to ask for them to make one more little function call in their
> constructors, before they upgrade to 5.10.  But we can’t do that if the
> function doesn't even exist yet in 5.9, to give them a grace period.  And
> 5.9 will be LTS… so the grace period will be quite long for those who
> need it to be.
> 
> The change in question is https://codereview.qt-project.org/#/c/193175/
> 
> My apologies for not noticing the need for this much earlier.
> 
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development