[Differential] [Commented On] D2820: Updating the wallpaper cache only when the configuration file settles

2016-09-29 Thread Ivan Čukić 
ivan added a comment.


  It works without the timer, but not without reparseConfiguration.
  
  I need to see why.

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #plasma, mart, davidedmundson
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D2820: Updating the wallpaper cache only when the configuration file settles

2016-09-28 Thread Ivan Čukić 
ivan added a comment.


  Well, I'm waiting for the response.
  
  I said that the point of the timer is not only to be able to tell that the 
file has been written, it is (like all the delay-timer tactics we are using 
everywhere) to flatten out multiple consecutive events.

INLINE COMMENTS

> broulik wrote in sortedactivitiesmodel.cpp:49
> Not needed?

Not needed. Was hoping I'll find a secret constructor that sets everything in 
the QTimer :)

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #plasma, davidedmundson, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D2820: Updating the wallpaper cache only when the configuration file settles

2016-09-22 Thread Ivan Čukić 
ivan added a comment.


  The timer is here so that we don't react if a couple of changes happen one 
after another, and go over the config to extract the wallpapers (just like 
having delayed config syncs in the fist place).
  
  While config (when shared) reloading is fast, going through the config is 
still not free.

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #plasma, davidedmundson, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D2820: Updating the wallpaper cache only when the configuration file settles

2016-09-21 Thread davidedmundson (David Edmundson) 
davidedmundson added inline comments.

INLINE COMMENTS

> sortedactivitiesmodel.cpp:197
>  bool initialized;
> -KConfig plasmaConfig;
> +KSharedConfig::Ptr plasmaConfig;
> +QTimer updateTimer;

Now it's a sharedconfig (a *very* sensible change) - you don't even need to 
call  reparseConfiguration().

that means it's fairly cheap to call reload.

that means you don't need the timer.

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #plasma, davidedmundson, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D2820: Updating the wallpaper cache only when the configuration file settles

2016-09-21 Thread mart (Marco Martin) 
mart added inline comments.

INLINE COMMENTS

> sortedactivitiesmodel.cpp:65
> +updateTimer.setSingleShot(true);
> +updateTimer.setInterval(500);
> +

is a blind timer the only way to wait enough? :/

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ivan, #plasma, davidedmundson, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas