fthx / dock-from-dash

GNU General Public License v3.0
96 stars 20 forks source link

workareas-changed signal is emitted "randomly", and makes the dock to be shown #35

Closed rastersoft closed 2 years ago

rastersoft commented 2 years ago

I have found that the dock, sometimes, shows "randomly" without I triggering it. After several tests, I found that the problem is that the "workareas-changed" signal is emitted sometimes when a new window is added or removed, and that forces the dock to show up.

The point is that I don't understand why are you using that signal...

rastersoft commented 2 years ago

Ok, now I see it: you use it to set the size and location of the dock.

I uploaded a MR that fixes this and other things.

fthx commented 2 years ago

Yes I do use this to detect resolution or monitor changes. There are caveats here and I took a lot of time to make it work reliably: e.g. triggering the overview allows the position to be really updated, for whatever GS signals update tricks.

fthx commented 2 years ago

There is a hide/show trick to trigger the position to be updated, too...

fthx commented 2 years ago

Note that I never experienced this bug!

fthx commented 2 years ago

https://github.com/fthx/dock-from-dash/commit/f99376f179e62f06bf653fb3c1faaf6f8eed7997 I'll look at the background bug, using 'always show'. I'll see if it happens once. (Was on X11? I do always use Wayland, even for tests so I cannot see this, if applicable).

fthx commented 2 years ago

Could you tell me how to reproduce, even if it's random?

rastersoft commented 2 years ago

It happens sometimes when I have three maximized windows in three workspaces, I close the last window (the one in the third workspace) and switch to the second: sometimes it is triggered there.

rastersoft commented 2 years ago

I use wayland, BTW.

fthx commented 2 years ago

Yes I can reproduce easily now. Just open a full screen w. Go to 2nd ws. Open a new full screen w. The signal is emitted.

I have an idea to workaround this bug in GS.

rastersoft commented 2 years ago

The old MR did a workaround by storing the size of the workspaces and comparing the new values with the old ones.

Anyway, maybe you should also separate the show part of the resize part.

fthx commented 2 years ago

really closed this time by: https://github.com/fthx/dock-from-dash/commit/db100649d3c70491115bde25b528e5e7b38ede6c

fthx commented 2 years ago

Anyway, maybe you should also separate the show part of the resize part.

It's possible but don't forget that dock has to be triggered once to make its position really updated in all situations. I'll be happy here if there is a smarter && shorter solution, but variables and signals in GS are sometimes lazy. It has to be tested at least when plugging in/out an external monitor, e.g., switching monitors...

rastersoft commented 2 years ago

But you can just update the size/position and avoid showing it...

rastersoft commented 2 years ago

(also, remember that sometimes you need to use "connect_after" instead of "connect"...)

fthx commented 2 years ago

If a connect_after (that I never used before) do avoid to trigger the dock (with show/hide trick), I'll be happy but it needs some tests because if we go for simply remove this trigger part, it won't update position correctly in all cases. What signal should I connect_after rather than connect?

rastersoft commented 2 years ago

I was talking about other signals, like "realize", where if you connect using "connect", you receive it "when the widget is going to be created in screen", but if you connect using "connect_after", you receive it "immediately after being created in the screen".

In this case, I think that you should separate dock_refresh() in two functions: one from line 189 to line 203, and another from line 204 to the end, and call the first one only when the size must be updated, and the second one only when the dock has to be presented on screen.

Also, I would connect _screen_border_box_refresh() to the "allocation-changed" signal of the dock; this way, every time the dock changes its size, the border box would also change it.

fthx commented 2 years ago

https://github.com/fthx/dock-from-dash/commit/cdadc7eab277393d6c48df65d1d1bde4fd73f375 We have to check if this version is reliable, especially for screen border box update when areas do change.

fthx commented 2 years ago

Also, I would connect _screen_border_box_refresh() to the "allocation-changed" signal of the dock; this way, every time the dock changes its size, the border box would also change it.

This does not work, as far as I tested.

rastersoft commented 2 years ago

Yes, I don't know why, but I receive an error if I try to connect to "allocation-changed"... which is odd.

But you can connect to "notify::width" and "notify::height" to the _box element, and use that to set the width and the height of the border box. I tested it and it does work.

fthx commented 2 years ago

Could you have a look at latest code here?

I've got one issue (only one?) here:

if (settings.get_boolean('always-show')) {
            this.set_position(this.work_area.x, this.work_area.y + this.work_area.height - this.height);
        } else {
            this.set_position(this.work_area.x, this.work_area.y + this.work_area.height);
        }

(line 146)

It does not work. The dock does not appear (autohide enabled). If I do simply: this.set_position(this.work_area.x, this.work_area.y + this.work_area.height - this.height); instead of this 'if' statement, it works but after a workareas refresh (toggled by GS bug) the position of the dock is 'shown position', so the animation is instant and that does not look clean.

fthx commented 2 years ago

If you're ok with that, we could look at the screenbox update with signal after having solved all the others known issues?

rastersoft commented 2 years ago

Mmm... I think that you are forgetting that the values of width and height aren't set from the beginning, but are being updated regularly based on constraints and the size of inner elements, so precisely connecting to the notify::width and notify::height signals and calling there a function that updates the position of the dock based on the current values is the only way of ensuring that everything works as expected.

rastersoft commented 2 years ago

Remember that any element begins with a size of 0x0, and only when it is realized are all its children called recursively to calculate the final size.

fthx commented 2 years ago

Ok but why in this case the one-line solution: this.set_position(this.work_area.x, this.work_area.y + this.work_area.height - this.height); does work?

rastersoft commented 2 years ago

Maybe because the border box isn't set correctly...?

fthx commented 2 years ago

But it is, if I just put the single command. I'm working on signals to update screen box, following your test.

fthx commented 2 years ago

I think this one is now good: https://github.com/fthx/dock-from-dash/commit/fc66e4d5b208c5bee8d75244ca3a30b9eaf01bab

fthx commented 2 years ago

the first position of the shown dock is not correct (seen better in always show mode). That's a problem I had a long time ago, and that's why I did use a show/hide trick.

If you have a solution?

fthx commented 2 years ago

https://github.com/fthx/dock-from-dash/commit/ba5c308e0d1c1f91792c41eb2abdae9900809ceb

Finally?...

rastersoft commented 2 years ago

I'll test it, give me some time.

fthx commented 2 years ago

Oh, that's not ok with always mode on. I maybe think we could remove this mode...

rastersoft commented 2 years ago

I think that the code is quite complex... I would do the following:

This way all the code is in a single place and is easier to keep track of all the possible cases...

fthx commented 2 years ago

https://github.com/fthx/dock-from-dash/commit/9172ebd6fbf1a60b5e2fce9551dd4e13d0c39549

rastersoft commented 2 years ago

I'll try it today

El lun., 4 abr. 2022 9:04, fthx @.***> escribió:

Closed #35 https://github.com/fthx/dock-from-dash/issues/35.

— Reply to this email directly, view it on GitHub https://github.com/fthx/dock-from-dash/issues/35#event-6360360668, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFMK55OLK4AY5UFDYG7IVLVDKIBTANCNFSM5SKAMVBA . You are receiving this because you authored the thread.Message ID: @.***>

rastersoft commented 2 years ago

There's still a problem: if there is no window in the current workspace, clicking on an icon makes the dock disappear. Only if there is already a window of another program will the dock remain after clicking.

fthx commented 2 years ago

https://github.com/fthx/dock-from-dash/commit/0cdc4587b016cdab58f02ebe1b9881b37552096d

fthx commented 2 years ago

GS extensions server is down but i'll push this soon.

rastersoft commented 2 years ago

It seems to be fixed now.