Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68400 --- looks like a useful change to me (and I hope it fixes the mispositioning I experienced today when restarting the system ;-) ) - Martin Gräßlin On Oct. 14, 2014, 6:15 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Oct. 14, 2014, 6:15 p.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68401 --- That seems to fix the panel showing on the wrong screen I was experiencing before. Nice. - Jeremy Whiting On Oct. 14, 2014, 10:15 a.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Oct. 14, 2014, 10:15 a.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68402 --- Ship it! Ship It! - Jeremy Whiting On Oct. 14, 2014, 10:15 a.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Oct. 14, 2014, 10:15 a.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68403 --- Ship it! Sounds sensible. however, can someone try plasma without this patch and with the Qt patch: https://codereview.qt-project.org/#/c/97050/ this one may be a workaround for the qt xcb problem adressed in the above, but i'm not 100% sure since i can't reproduce the issue of panel starting up on wrong monitor (even if is a workaround, this one may be needed anyways since it would take aeons anyways for the qt patch making to releases) - Marco Martin On Ott. 14, 2014, 4:15 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Ott. 14, 2014, 4:15 p.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
> On Ott. 14, 2014, 4:36 p.m., Marco Martin wrote: > > Sounds sensible. > > however, can someone try plasma without this patch and with the Qt patch: > > https://codereview.qt-project.org/#/c/97050/ > > > > this one may be a workaround for the qt xcb problem adressed in the above, > > but i'm not 100% sure since i can't reproduce the issue of panel starting > > up on wrong monitor (even if is a workaround, this one may be needed > > anyways since it would take aeons anyways for the qt patch making to > > releases) even without the patch in qt, to see if it was *that* issue, it can be checked if seeting the panels as window can cover (or anything *without* struts) makes the problem not happen. so the problem is the following: * the panel sets an extended strut, removing its geometry from QScreen::AvailableGeometry() * if the panel is ill-positioned just a little and overflows in the second screen even just a pixel (admittedly a bug in itself, but unrelated), qt xcb sees that the panel is not in the availableGeometry() of its screen, so checks if it is of any other screen, and if it is of even 1 pixel, it migrates the panel t the new screen - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68403 --- On Ott. 14, 2014, 4:15 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Ott. 14, 2014, 4:15 p.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/ --- (Updated Oct. 15, 2014, 1:54 p.m.) Review request for Plasma. Changes --- I took another go at the problem because I realized that this patch only fixed the issue if I had the panel laid out horizontally. Reasoning:Consider 2 screens laid out horizontally, we want to have the panel at the right screen, vertically at the right. (1280x800 and 1920x1080) The problem is probably on KWin (or somewhere outside plasma anyway) because we would put the panel at 0x0, then set the window to the geometry and screen. What QtXcb was receiving was: 0: QRect(0,0 12x12) 1: QRect(3146,0 53x1080) << which is taking the wrong window width 2: QRect(1226,0 53x1080) 0 is the initial window size, 1 happened when reparenting the containment into the panel. Then given that 1 wasn't considered to be on any screen (note that 3146-1920-1280=-54, and we needed 53), then the window was moved to the primaryScreen. Because Qt knows best <3. Now this patch makes sure we're initializing the window in the correct place, so that Qt needs to do less magic, by positioning the panel before reparenting the containment QQuickItem into the panel, which is when things started to move around. Also I saw that we were initializing the position considering the 12x12 instead of the panel thickness, which was causing also this window kicking. Use the proper size. Bugs: 339672 https://bugs.kde.org/show_bug.cgi?id=339672 Repository: plasma-workspace Description --- I was getting the Panels wrongly positioned because Qt would reset the position at some point while initializing the class. This patch disables any position while the panel is not shown and makes sure it's placed when it's set to be shown. Diffs (updated) - shell/panelview.cpp 9260c18 shell/shellcorona.cpp ba66d46 Diff: https://git.reviewboard.kde.org/r/120584/diff/ Testing --- Both my installations initialize properly now. Thanks, Aleix Pol Gonzalez ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68453 --- Ship it! Inviala! - Marco Martin On Ott. 15, 2014, 1:54 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Ott. 15, 2014, 1:54 p.m.) > > > Review request for Plasma. > > > Bugs: 339672 > https://bugs.kde.org/show_bug.cgi?id=339672 > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > shell/shellcorona.cpp ba66d46 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68455 --- I consider it as highly unlikely that KWin positioned the panel unless it (or Qt) was doing something wrong. The panel should request a position and KWin honors that and as it's a panel there is nothing which should make KWin in applying any rules. Anyway: if you have the feeling it could be KWin I recommend to pick a different window manager for the test (e.g. OpenBox). If you see the same or similar results one can rule out the WM. - Martin Gräßlin On Oct. 15, 2014, 3:54 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Oct. 15, 2014, 3:54 p.m.) > > > Review request for Plasma. > > > Bugs: 339672 > https://bugs.kde.org/show_bug.cgi?id=339672 > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > shell/shellcorona.cpp ba66d46 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
> On Oct. 15, 2014, 2:06 p.m., Martin Gräßlin wrote: > > I consider it as highly unlikely that KWin positioned the panel unless it > > (or Qt) was doing something wrong. The panel should request a position and > > KWin honors that and as it's a panel there is nothing which should make > > KWin in applying any rules. > > > > Anyway: if you have the feeling it could be KWin I recommend to pick a > > different window manager for the test (e.g. OpenBox). If you see the same > > or similar results one can rule out the WM. Correct, that was written before I changed the positionPanel width()->thickness(). Sorry about that, my fault. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/#review68455 --- On Oct. 15, 2014, 1:54 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120584/ > --- > > (Updated Oct. 15, 2014, 1:54 p.m.) > > > Review request for Plasma. > > > Bugs: 339672 > https://bugs.kde.org/show_bug.cgi?id=339672 > > > Repository: plasma-workspace > > > Description > --- > > I was getting the Panels wrongly positioned because Qt would reset the > position at some point while initializing the class. This patch disables any > position while the panel is not shown and makes sure it's placed when it's > set to be shown. > > > Diffs > - > > shell/panelview.cpp 9260c18 > shell/shellcorona.cpp ba66d46 > > Diff: https://git.reviewboard.kde.org/r/120584/diff/ > > > Testing > --- > > Both my installations initialize properly now. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120584: Don't position the panel until it's about to be displayed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120584/ --- (Updated Oct. 15, 2014, 3:15 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Bugs: 339672 https://bugs.kde.org/show_bug.cgi?id=339672 Repository: plasma-workspace Description --- I was getting the Panels wrongly positioned because Qt would reset the position at some point while initializing the class. This patch disables any position while the panel is not shown and makes sure it's placed when it's set to be shown. Diffs - shell/panelview.cpp 9260c18 shell/shellcorona.cpp ba66d46 Diff: https://git.reviewboard.kde.org/r/120584/diff/ Testing --- Both my installations initialize properly now. Thanks, Aleix Pol Gonzalez ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
