D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> zzag wrote in windowthumbnail.cpp:213
> Do you actually need Q_FALLTHROUGH() here?

I fall through so I wanted to make it explicit. I would have put a `// 
fallthrough` comment otherwise

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20805

To: broulik, #plasma, davidedmundson
Cc: zzag, kde-frameworks-devel, michaelh, ngraham, bruns


D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread Vlad Zagorodniy
zzag added inline comments.

INLINE COMMENTS

> windowthumbnail.cpp:213
> +case ItemEnabledHasChanged:
> +Q_FALLTHROUGH();
> +case ItemVisibleHasChanged:

Do you actually need Q_FALLTHROUGH() here?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20805

To: broulik, #plasma, davidedmundson
Cc: zzag, kde-frameworks-devel, michaelh, ngraham, bruns


D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:1b2424879a19: [Window Thumbnail] Also monitor scene 
visibility and clean up (authored by broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D20805?vs=56951=56956#toc

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20805?vs=56951=56956

REVISION DETAIL
  https://phabricator.kde.org/D20805

AFFECTED FILES
  src/declarativeimports/core/windowthumbnail.cpp
  src/declarativeimports/core/windowthumbnail.h

To: broulik, #plasma, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread David Edmundson
davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20805

To: broulik, #plasma, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread Kai Uwe Broulik
broulik added a comment.


  I recall there was also a bug about "games becoming slow when hovering them 
in task bar" which might be the same bug: us not unredirecting it again

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20805

To: broulik, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D20805: [Window Thumbnail] Also monitor scene visibility and clean up

2019-04-25 Thread Kai Uwe Broulik
broulik created this revision.
broulik added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Just because the item is `visible` doesn't mean the window itself is. Keep 
track of the window it's in.
  Use `itemChange` instead of connects and move the check for starting to 
`startRedirect` so we don't have to check the conditions for that in every 
change handler (visible && enabled && window->visible). It also saves three 
connects in the constructor.
  Also, don't unredirect if we didn't redirect in the first place to avoid 
warnings printed on console.
  
  BUG: 406303

TEST PLAN
  Window thumbnails still work
  Don't have fds pile up anymore

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20805

AFFECTED FILES
  src/declarativeimports/core/windowthumbnail.cpp
  src/declarativeimports/core/windowthumbnail.h

To: broulik, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns