minimize dialog resizes/moves
ClosedPublic

Authored by mart on May 18 2017, 2:10 PM.

Details

Summary

take into account size hints also when adjusting with synctomainitemsize
which sometimes has to be executed right before adjusting from the layout
hints, giving one wrong resize

introduce geometryUpdatesBlocked, which stops the dialog
from syncing which is useful when both visual parent and item
contents gets updated in one go (the tooltip) it is not
yet exported to qml and it shouldn't as is dangerous, but
kicker may make use of it between changing the submenu model and
the visualparent

alternative implementations may be:

  • a method that takes both main item and visual parent
  • delaying with a timer setGeometry (ouch)

this part can also be left behind for now and see if the rest is good enough

Test Plan

no visual glitches visible anymore in wayland when moving the tooltip
in the taskbar, x11 ok too

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.May 18 2017, 2:10 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptMay 18 2017, 2:10 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
davidedmundson added inline comments.
src/plasmaquick/dialog.cpp
860

I don't understand your description of the problem.

this call both that resizes and moves.
but if the item hasn't changed yet, it shouldn't actually resize anything.

you're implying it does. Why?

mart added inline comments.May 18 2017, 6:50 PM
src/plasmaquick/dialog.cpp
860
  • setMainItem -> the main item will change, the Layout attached property too (or get null, or go from null from something)
  • syncToMainItemSize -> resizes the dialog to the main item size, which may or may not be currently in a size between the minimum/maximum hints bounds
  • updateLayoutParameters finally resizes it into bounds if it wasn't, so, one resize too much

actually, seems to still be one too much, down from two too much, as sometimes seems the layout will adjust its size hints only a while after the qquickitem has been actually drawn, which makes this last one a bit hard to remove

mart updated this revision to Diff 14688.May 19 2017, 12:51 PM
  • remove updategeometryblocked
mart added a comment.May 19 2017, 1:00 PM

removed that extra complexity block/unblock that didn't seem to buy anything.

comparing before/after, all the normal tooltips, exept taskmanager consistyently passed from two resizes to one.
perhaps will be possible to fix the taskmanager case by tweaking up how its tooltip exports size hints

mart updated this revision to Diff 14690.May 19 2017, 1:39 PM
  • make the tooltips actual tooltips
mart added a comment.May 19 2017, 1:40 PM

now tooltips are animated again

hein accepted this revision.May 24 2017, 4:08 PM
This revision is now accepted and ready to land.May 24 2017, 4:08 PM
davidedmundson requested changes to this revision.May 24 2017, 4:11 PM
davidedmundson added inline comments.
src/declarativeimports/core/tooltipdialog.cpp
40

Calling set flags explicitly and set type explicitly is conflicting.

This revision now requires changes to proceed.May 24 2017, 4:11 PM
mart updated this revision to Diff 14809.May 24 2017, 4:22 PM
mart edited edge metadata.
  • use settype, not setflags
mart added inline comments.May 24 2017, 4:25 PM
src/declarativeimports/core/tooltipdialog.cpp
40

now just uses settype, tested on x11, still works and tooltip windows still has _NET_WM_WINDOW_TYPE_TOOLTIP
settype is needed in wayland as it uses the plasmashell protocol to set it as a parentless tooltip

broulik added inline comments.
src/plasmaquick/dialog.cpp
487

Those asserts are redundant now

Offtopic, but in future can you adjust the summary when rewriting a patch rather than leaving a comment, it's easier to read. Phab still keeps the history if we need it.

So this is for fixing when we have a mainItem whose size is bigger/smaller than its own size hints?
We resize the main itemin updateLayoutParams() but we've already resized the window by then.

If I've understood that correctly, that makes sense, and this is an improvement.

but: we're still doing a resize whislt we have the old window management min/max hints set from the previous item, so it'll still have an extra resize, just at a kwin level not here.

src/plasmaquick/dialog.cpp
301–306

From what I can tell, you're basically trying to put the code from updateLayoutParameters in to synctoMainItemSize which is fine, but then we shouldn't end up calling both.

davidedmundson accepted this revision.May 24 2017, 7:45 PM
This revision is now accepted and ready to land.May 24 2017, 7:45 PM
This revision was automatically updated to reflect the committed changes.
mart added a comment.May 25 2017, 9:57 AM

but: we're still doing a resize whislt we have the old window management min/max hints set from the previous item, so it'll still have an extra resize, just at a kwin level not here.

where are you getting them? now, i still i get an extra resize when switching maintitem in taskbar tooltips, not on other tooltips that share the same mainitem