D17006: Compress calls to `updateSize`

[Victor Ryzhykh]

victorr added a comment.


  F6441215: group.jpeg 
  Now the group looks like this.
  After clicking on it, the plasmashell falls.
  You can check it on unstable KDE 
https://files.kde.org/neon/images/neon-devedition-gitunstable/current/
  after updating from
  
https://origin.archive.neon.kde.org/dev/unstable/pool/main/p/plasma-desktop/?C=M;O=D

REPOSITORY
  R119 Plasma Desktop

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

To: hein, #plasma, davidedmundson
Cc: victorr, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17006: Compress calls to `updateSize`

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:ab26ebb18b74: Compress calls to `updateSize` (authored by 
hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17006?vs=45780=45785#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17006?vs=45780=45785

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/GroupDialog.qml
  containments/desktop/package/contents/ui/FolderItemDelegate.qml

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17006: Compress calls to `updateSize`

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> hein wrote in GroupDialog.qml:265
> The suggested code is the same as the present code, so not sure ... can you 
> clarify?
> 
> The aboutToPopulate thing is done for the focus-active-task. I'm not sure I 
> want to replace this with `callLater`, since callLater has weakly defined 
> semantics for me tastes. For the `updateSize` case only eventual consistency 
> counts. If it was executed too early and then later again it'd at worst be 
> more like the old performance, but not break things. With the 
> focus-active-task code, though, it could break things.

My line changes

  || -> &&

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17006: Compress calls to `updateSize`

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> davidedmundson wrote in GroupDialog.qml:265
> I remember looking into this.
> 
> I was thinking this should be:
> 
> (!groupRepeater.aboutToPopulate && visualParent.childCount == 
> groupRepeater.count)
> 
> As if we're still aboutToPopulate then we shouldn't be doing anything.
> 
> But then I changed it and it didn't immediately work so I did nothing.

The suggested code is the same as the present code, so not sure ... can you 
clarify?

The aboutToPopulate thing is done for the focus-active-task. I'm not sure I 
want to replace this with `callLater`, since callLater has weakly defined 
semantics for me tastes. For the `updateSize` case only eventual consistency 
counts. If it was executed too early and then later again it'd at worst be more 
like the old performance, but not break things. With the focus-active-task 
code, though, it could break things.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17006: Compress calls to `updateSize`

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  Use of callLater seems like a neat solution, does it make aboutToPopulate 
redundant?

INLINE COMMENTS

> GroupDialog.qml:265
>  // only update size once the repeater count matches the model role.
>  } else if (!groupRepeater.aboutToPopulate || visualParent.childCount 
> == groupRepeater.count) {
>  var task;

I remember looking into this.

I was thinking this should be:

(!groupRepeater.aboutToPopulate && visualParent.childCount == 
groupRepeater.count)

As if we're still aboutToPopulate then we shouldn't be doing anything.

But then I changed it and it didn't immediately work so I did nothing.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17006: Compress calls to `updateSize`

[Eike Hein]

hein created this revision.
hein added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION SUMMARY
  Repeater+Flow has no reliable way to transactionize/batch insertion,
  e.g. `Flow.positionCompleted` is useless in a function that can change
  the Flow's size, and transition events like populated and similar
  aren't available. Refactoring this to be a ListView isn't appealing
  because it would mean the code loses the property of being reused for
  both the bar and the popup, and therefore is well-tested already.
  
  This brings down the number of `updateSize` calls when opening the
  dialog for a group of five from five to one.
  
  BUG:400364

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/GroupDialog.qml

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart