Propagate more unused events in edit tools
Changes PlannedPublic

Authored by rkflx on Aug 16 2018, 8:02 PM.


Group Reviewers

In RasterImageView events related to AbstractRasterImageViewTool are
propagated to the parent class in case they have not been accepted. In
case a tool derived from AbstractRasterImageViewTool does not
implement its own handler for an event, the empty handler of
AbstractRasterImageViewTool is used. Since "by default isAccepted()
is set to true" according to the QEvent docs, this then results in
events not being propagated.

This can lead to issues, as demonstrated in D14286. As a follow-up to
that and to D14755, let's ignore() events by default in
AbstractRasterImageViewTool, so they are passed on.

Depends on D14485

Test Plan

Invoke Crop and Reduce Red Eye, and test keyboard handling,
mouse-hovering and clicking for the tool's functions as well as generic
functions like zooming and panning. Everything should work as before.

Diff Detail

R260 Gwenview
tool-events-cleanup (branched from master)
No Linters Available
No Unit Test Coverage
Build Status
Buildable 1986
Build 2004: arc lint + arc unit
rkflx requested review of this revision.Aug 16 2018, 8:02 PM
rkflx created this revision.

I found an issue with the ignored keyReleaseEvent - pressing and releasing Ctrl changes the cursor to open or closed hand. For Crop this corrects when the mouse is moved but not for RedEye. Maybe we could solve this in combination with D14485 or just override keyReleaseEvent for now?

There are only three events which are not overridden by the two derived classes and could have changed something:

  • wheelEvent: correctly handled by zoom
  • keyReleaseEvent: passed to AbstractImageView::keyReleaseEvent()
  • hoverMoveEvent (RedEyeReductionTool only): used by BirdEyeView and still works good
rkflx updated this revision to Diff 39985.Aug 18 2018, 10:58 PM
rkflx edited the summary of this revision. (Show Details)

Ah, right. So doing the right thing here actually uncovers hidden problems elsewhere. Since you already solved them in
D14485, I'll depend on your Diff here.

  • rebase on D14485
  • add focusInEvent
  • fix pointer formatting
rkflx updated this revision to Diff 39987.Aug 18 2018, 11:04 PM

(Please ignore the previous attempt ;)

rkflx planned changes to this revision.Aug 25 2018, 8:34 AM
Restricted Application added a project: Gwenview. · View Herald TranscriptAug 25 2018, 8:34 AM