Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: dock alignment doesn't work after setting initial state to withdrawn #2112

Open
Bluey26 opened this issue Dec 8, 2024 · 9 comments
Open
Assignees
Labels
display: x11 related to X11 backend regression something that worked previously but was broken at some point by some commit struts related to conky reserving space along screen edge

Comments

@Bluey26
Copy link

Bluey26 commented Dec 8, 2024

What happened?

For conky versions newer than 1.21.7 there seems to be a X11 regression in where the conky window is not showing itself anymore in the given position. alignment = 'top_right' seems to be ignored, and the window is shown in the top_left border, transparency is not detected anymore (own_window_argb_value = 90 ignored) and there is also a "windowing" issue, since the own_window_type = 'dock' property is also ignored (the conky panel "reserves" the space and avoids other windows and the desktop itself to be on top of it.

I discovered this after compiling the lastest changes in conky, but it seems to be a problem arisen from prior changes. Downgrading my conky to conky-git-1.21.7.r5.g71814776-1 seems to solve the issue, for some reason (x11 backend now has all the mentioned properties working).

This seems to be independent of the wayland_support argument. For some reason, the issue is not shown when using the wayland backend in a Wayland session (i.e. labwc)

PS: I don't think that my conky config is relevant in this case, because the .conf is the same for the wayland backend(1.21.9) and for the previous version (1.21.7), in both cases giving me a working conky.

PS2: Since there is a WIP wayland_backend that works now, i was thinking if it would be possible to have a conky property that lets the user to have a "preference" over the backend use, something like : prefer_backend = wayland or prefer_wayland = 1. This is an issue because if somebody uses x11 and wayland from time to time, out_to_wayland = true, out_to_x = true, will give the x11 (xwayland) backend in wayland instead of the newer wayland backend.

Version

Lastest (1.21.9)

Which OS/distro are you seeing the problem on?

Arch Linux

Conky config

No response

Stack trace

No response

Relevant log output

No response

@Bluey26 Bluey26 added bug related to incorrect existing implementation of some functionality triage issue that hasn't been verified, categorized or acknowledged yet labels Dec 8, 2024
@Caellian Caellian added display: x11 related to X11 backend struts related to conky reserving space along screen edge and removed triage issue that hasn't been verified, categorized or acknowledged yet labels Dec 8, 2024
@Caellian
Copy link
Collaborator

Caellian commented Dec 8, 2024

Related PR in 1.21.8 is #2056.
But I believe that's not the cause of the issue and only unearths some other problem.
Can you check whether you're experiencing issue described in #2046 on 1.21.7?

Commits that touch set_struts:

@Bluey26
Copy link
Author

Bluey26 commented Dec 8, 2024

Can you check whether you're experiencing issue described in #2046 on 1.21.7?

If i understand the issue correctly, i should see the conky dock window over all the windows, in "top" of them.

That does not happen in my build,fortunately.

I was looking at the compiled package i have for conky 1.21.7 and it was compiled on 2024-09-17 (17th Sept), thus it should be the same as : 7181477

This means my package is not the 1.21.7 release, but a middle between 1.21.7 and 1.21.8.

The change that may be triggering my issue is somewhat recent, something after 17th september.

I will compile that commit again and see what happens with the compiled package, just to discard some kind of compilation issue(newer dependencies that may cause the issue mentioned in here).

Update: The compiled package from that commit works as expected, i will try one newer.

Update 2: I compiled the commit 9e265a3
and the issue arises. So this means the conflicting commit is between 17th september and 5th November.

@Bluey26
Copy link
Author

Bluey26 commented Dec 9, 2024

I compiled a intermediate commit (ebef880) which seems to have a regression regards the issue. This commit has the position issue, and seems likely to be the one involved ("fix incorrect x11 dock/panel placement").

This has introduced the regression, at least in here.

Update: I double checked, b05810d does not have the issue (its the previous commit).

@Caellian
Copy link
Collaborator

Caellian commented Dec 9, 2024

Right, but that change reverted that code as is were before I touched that code, which is why I said ebef880 should be ignored. I commented it out as part of 6adf6b9 (here), by accident.

What's worth comparing is the set_struts implementation and changes in used variables.

Original implementation

/* reserve window manager space */
void set_struts(int sidenum) {
  Atom strut;
  if ((strut = ATOM(_NET_WM_STRUT)) != None) {
    /* reserve space at left, right, top, bottom */
    signed long sizes[12] = {0};
    int i;

    /* define strut depth */
    switch (sidenum) {
      case 0:
        /* left side */
        sizes[0] = window.x + window.width;
        break;
      case 1:
        /* right side */
        sizes[1] = display_width - window.x;
        break;
      case 2:
        /* top side */
        sizes[2] = window.y + window.height;
        break;
      case 3:
        /* bottom side */
        sizes[3] = display_height - window.y;
        break;
    }

    /* define partial strut length */
    if (sidenum <= 1) {
      sizes[4 + (sidenum * 2)] = window.y;
      sizes[5 + (sidenum * 2)] = window.y + window.height;
    } else if (sidenum <= 3) {
      sizes[4 + (sidenum * 2)] = window.x;
      sizes[5 + (sidenum * 2)] = window.x + window.width;
    }

    /* check constraints */
    for (i = 0; i < 12; i++) {
      if (sizes[i] < 0) {
        sizes[i] = 0;
      } else {
        if (i <= 1 || i >= 8) {
          if (sizes[i] > display_width) { sizes[i] = display_width; }
        } else {
          if (sizes[i] > display_height) { sizes[i] = display_height; }
        }
      }
    }

    XChangeProperty(display, window.window, strut, XA_CARDINAL, 32,
                    PropModeReplace, reinterpret_cast<unsigned char *>(&sizes),
                    4);

    if ((strut = ATOM(_NET_WM_STRUT_PARTIAL)) != None) {
      XChangeProperty(display, window.window, strut, XA_CARDINAL, 32,
                      PropModeReplace,
                      reinterpret_cast<unsigned char *>(&sizes), 12);
    }
  }
}

I need to write a test for it and figure out what I broke.

@Caellian Caellian self-assigned this Dec 9, 2024
@Caellian Caellian changed the title [Bug]: Issues with x11 backend in newer Conky versions [Bug]: dock alignment doesn't work after setting initial state to withdrawn Dec 9, 2024
@Caellian
Copy link
Collaborator

Caellian commented Dec 9, 2024

I'm working on adding tests for set_struts in #2115.

@Caellian
Copy link
Collaborator

Which WM are you on?

@Bluey26
Copy link
Author

Bluey26 commented Dec 10, 2024

Which WM are you on?

I use labwc, which is based in wlroots.

In x11, i use openbox

@Caellian
Copy link
Collaborator

That's odd, the current set_struts implementation should work with openbox because it adheres to that standard. I'll probably need to compare our code with what tint2 does and hope we don't have to switch between #2056 and commented out version based on WM.

I tested this code on openbox and commented out version worked correctly, it's possible that the uncommented one didn't which is why I could've left it commented out. Just X11 things I guess... hahah

There should exist a version that works for both without having to check for WM.

fluxbox doesn't adhere to the spec with regards to struts and #2056 fixes this code for that WM (and probably compiz, kwin, i3 as well).

@Bluey26
Copy link
Author

Bluey26 commented Dec 17, 2024

In my opinion it would be useful to have such option in the meanwhile. Moreover, if the releases do not have those, there may be issues with stable Linux distributions which still are using Openbox as WM.

I'm using wayland daily so its not affecting me that much, plus i can always use a previous package.

@Caellian Caellian added regression something that worked previously but was broken at some point by some commit and removed bug related to incorrect existing implementation of some functionality labels Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display: x11 related to X11 backend regression something that worked previously but was broken at some point by some commit struts related to conky reserving space along screen edge
Projects
None yet
Development

No branches or pull requests

2 participants