Merge lp:~townsend/compiz/fix-snap-window-resize-0.9.10 into lp:compiz/0.9.10

Proposed by Christopher Townsend
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 3795
Merged at revision: 3796
Proposed branch: lp:~townsend/compiz/fix-snap-window-resize-0.9.10
Merge into: lp:compiz/0.9.10
Diff against target: 81 lines (+0/-71)
1 file modified
plugins/snap/src/snap.cpp (+0/-71)
To merge this branch: bzr merge lp:~townsend/compiz/fix-snap-window-resize-0.9.10
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+189157@code.launchpad.net

Commit message

Remove seemingly useless code in the SnapWindow::resizeNotify() method where a snapped window that is being resized on the opposite side of the snap would cause orders of magnitude more calls to window->resize() and kill performance.

Description of the change

= Issue =
Set Default Resizing mode to Normal. Snap a window to an edge such as the Launcher or the Panel or screen edge. Resize the opposite side of the snapped edge. Terrible performance ensues.

= Fix =
A seemingly useless code block in the SnapWindow::resizeNotify() method would make orders of magnitude more calls to window->resize() in this case. Each call destroys/creates a new X Window. For example a normal resize may call resize 5 times, but with this code in place, it may call 5000 times or more.

= Note =
I could not figure out what the real intent of the code block was for when resizing a window. It just didn't seem like it was supposed to do anything. As a matter of fact, when using the Default Resizing mode as Rectangle, this code was never touched, so it's not as if resizing a snapped window needed to do something special. At any rate, some pretty good testing of various resizing should be done to ensure there is no regressions by removing this code.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

LGTM

But would like someone else to test it out :)

review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

This seems to work well here, I was jut worried by the possibility that this could lead to non working snapping when resizing a window (if enabled), but I don't see that working even in normal version... So I think this is fine

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/snap/src/snap.cpp'
2--- plugins/snap/src/snap.cpp 2013-05-09 13:43:07 +0000
3+++ plugins/snap/src/snap.cpp 2013-10-03 18:19:48 +0000
4@@ -587,77 +587,6 @@
5 if (!ss->snapping)
6 return;
7
8- // apply edge resistance
9- if (ss->optionGetSnapTypeMask () & SnapTypeEdgeResistanceMask)
10- {
11- // If there's horizontal snapping, add dx to current buffered
12- // dx and resist (move by -dx) or release the window and move
13- // by buffered dx - dx, same for dh
14- if (!snapGeometry.isEmpty () && snapDirection & HorizontalSnap)
15- {
16- m_dx += dx;
17- if (m_dx < ss->optionGetResistanceDistance ()
18- && m_dx > -ss->optionGetResistanceDistance ())
19- {
20- resize (-dx, 0, 0, 0);
21- }
22- else
23- {
24- resize (m_dx - dx, 0, 0, 0);
25- m_dx = 0;
26- if (!m_dwidth)
27- snapDirection &= VerticalSnap;
28- }
29- m_dwidth += dwidth;
30- if (m_dwidth < ss->optionGetResistanceDistance ()
31- && m_dwidth > -ss->optionGetResistanceDistance ())
32- {
33- resize (0, 0, -dwidth, 0);
34- }
35- else
36- {
37- resize (0, 0, m_dwidth - dwidth, 0);
38- m_dwidth = 0;
39- if (!m_dx)
40- snapDirection &= VerticalSnap;
41- }
42- }
43-
44- // Same for vertical snapping and dy/dh
45- if (snapGeometry.isEmpty () && snapDirection & VerticalSnap)
46- {
47- m_dy += dy;
48- if (m_dy < ss->optionGetResistanceDistance ()
49- && m_dy > -ss->optionGetResistanceDistance ())
50- {
51- resize (0, -dy, 0, 0);
52- }
53- else
54- {
55- resize (0, m_dy - dy, 0, 0);
56- m_dy = 0;
57- if (!m_dheight)
58- snapDirection &= HorizontalSnap;
59- }
60- m_dheight += dheight;
61- if (m_dheight < ss->optionGetResistanceDistance ()
62- && m_dheight > -ss->optionGetResistanceDistance ())
63- {
64- resize (0, 0, 0, -dheight);
65- }
66- else
67- {
68- resize (0, 0, 0, m_dheight - dheight);
69- m_dheight = 0;
70- if (!m_dy)
71- snapDirection &= HorizontalSnap;
72- }
73- }
74- // If we are no longer snapping in any direction, reset snapped
75- if (!snapGeometry.isEmpty () && !snapDirection)
76- snapGeometry = CompWindow::Geometry ();
77- }
78-
79 // If we don't already snap vertically and horizontally,
80 // check edges status
81 if (snapDirection != (VerticalSnap | HorizontalSnap))

Subscribers

People subscribed via source and target branches