[shell] Event compress resizePanel
AbandonedPublic

Authored by bshah on Oct 7 2016, 8:47 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Summary

At startup and as well when one moves panel, resizePanel is called
multiple times. For instance, at startup resizePanel is called 8 times
in row from different callers.

So add timer to do panel resize in one go. This will avoid multiple
resize.

Test Plan

Resized, and moved my panel, worked without any obvious regressions. also added
debug statement in the resizePanel, which got printed just 3 times at startup
with this patch while before it was getting printed 8-9 times.

Diff Detail

Repository
R120 Plasma Workspace
Branch
event-compress-panel-resize (branched from Plasma/5.8)
Lint
No Linters Available
Unit
No Unit Test Coverage
bshah updated this revision to Diff 7184.Oct 7 2016, 8:47 AM
bshah retitled this revision from to [shell] Event compress resizePanel.
bshah updated this object.
bshah edited the test plan for this revision. (Show Details)
bshah added a reviewer: Plasma.
Restricted Application added a project: Plasma. · View Herald TranscriptOct 7 2016, 8:47 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
hein added a subscriber: hein.Oct 7 2016, 9:10 AM

100% no. Timers with magic timeout values are a "let's hope it works out if we wait a bit" anti-pattern to paper over figuring out proper update logic.

mart added a subscriber: mart.Oct 7 2016, 9:48 AM

can it use QQmlParserStatus instead?

bshah added a comment.Oct 7 2016, 9:58 AM
In D2975#55224, @mart wrote:

can it use QQmlParserStatus instead?

Idea of using QQmlParserStatus is fine and it might save some cycles at start but problem remains same when I move panel around.. I don't think QQmlParser status can help there..(?)

mart added a comment.Oct 10 2016, 11:48 AM

adding an event compression timer (that by the way, it's a perfectly fine, many times even desired thing) and solving the useless restore() calls at startup are two ortogonal pissues.
an event compression there *may* be a good thing if during the normal use (like, dragging panels around) things like setLength() still get called too much.
but right now as the main problem is the startup issue, is too soon to put this in, as it hides the real problem

In D2975#55706, @mart wrote:

adding an event compression timer (that by the way, it's a perfectly fine, many times even desired thing) and solving the useless restore() calls at startup are two ortogonal pissues.

Double restore() issue was fixed by other commits..

an event compression there *may* be a good thing if during the normal use (like, dragging panels around) things like setLength() still get called too much.

In my testing, moving panel from bottom edge to right edge calls resizePanel 78 times.. and moving between two screens it calls it 179 times, IMO this are high numbers to require event compression.

but right now as the main problem is the startup issue, is too soon to put this in, as it hides the real problem

I agree that we will need to solve startup issue, probably using QQmlParserStatus at proper place..

mart added a comment.Oct 10 2016, 12:09 PM

ok, so let's try to do the QQmlParserStatus thing (can you give a stab at it?) then when it's working and we know solves the startup issues, we reconsider this one again?

In D2975#55709, @mart wrote:

ok, so let's try to do the QQmlParserStatus thing (can you give a stab at it?) then when it's working and we know solves the startup issues, we reconsider this one again?

Okay looking onto it.

bshah added a comment.Oct 10 2016, 4:15 PM
In D2975#55717, @bshah wrote:
In D2975#55709, @mart wrote:

ok, so let's try to do the QQmlParserStatus thing (can you give a stab at it?) then when it's working and we know solves the startup issues, we reconsider this one again?

Okay looking onto it.

In c63baa3ae , panel resize and reposition at startup is fixed.

broulik added a subscriber: broulik.Mar 2 2017, 8:32 PM

Is this obsolete then?

davidedmundson requested changes to this revision.Nov 16 2018, 11:53 AM
davidedmundson added a subscriber: davidedmundson.

Marking as request changes to remove from queue pending reply to Kai's question

This revision now requires changes to proceed.Nov 16 2018, 11:53 AM
bshah abandoned this revision.Nov 16 2018, 12:19 PM