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

Fix out_to_wayland giving nothing #2103

Merged
merged 2 commits into from
Dec 7, 2024

Conversation

stacyharper
Copy link
Contributor

@stacyharper stacyharper commented Dec 7, 2024

This fix a problem caused by 6adf6b9:

The display outputs are sorted by priority on src/display-output.cc.

Before the cleanup commit, Wayland was choosed instead of X11. But no thanks to any explicit code. Just because outputs was inserted luckily in the correct order, before the sort.

Because out_to_x is true, and out_to_wayland is false by default, it means the users have to enable explicitely wayland on their config.

We should rather prefer Wayland because it need explicit configuration.

To do so, I think we should define that Wayland priority surpass the X11 one, instead of fixing the register_output order.

(There is yet another issue with Wayland output at the moment (caused by #1876 (comment)). I'm investigating it. And hopefully, two patch are needed to fix most of the Wayland related tickets)

Checklist

  • I have described the changes
  • I have linked to any relevant GitHub issues, if applicable
  • Documentation in doc/ has been updated
  • All new code is licensed under GPLv3

This fix a problem caused by 6adf6b9:

The display outputs are sorted by priority on src/display-output.cc.

Before the cleanup commit, Wayland was choosed instead of X11. But
no thanks to any explicit code. Just because outputs was inserted
luckily in the correct order, before the sort.


Because out_to_x is true, and out_to_wayland is false by default,
it means the users have to enable explicitely wayland on their config.

We should rather prefer Wayland because it need explicit configuration.


To do so, I think we should define that Wayland priority surpass the X11
one, instead of fixing the register_output order.
@github-actions github-actions bot added sources PR modifies project sources display: wayland related to Wayland backend labels Dec 7, 2024
Copy link

netlify bot commented Dec 7, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit d696b33
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/67549c974c87490008cc709c

@brndnmtthws
Copy link
Owner

Sounds reasonable to me, but I will defer to @Caellian on this who knows the Wayland details better than me.

@github-actions github-actions bot added display: x11 related to X11 backend display: ncurses related to ncurses backend display: file related to file backend display: http related to HTTP backend display: console related to console backend labels Dec 7, 2024
@Caellian Caellian added the bug related to incorrect existing implementation of some functionality label Dec 7, 2024
Copy link
Collaborator

@Caellian Caellian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for tracking this down, and sorry for inadvertently breaking things.

Comment on lines 84 to 98
// Order of registration is important!
// - Graphical outputs go before textual (e.g. X11 before NCurses).
// - Optional outputs go before non-optional (e.g. Wayland before X11).
// - Newer outputs go before older (e.g. NCurses before (hypothetical) Curses).
// - Fallbacks go last (in group)
register_output<output_t::WAYLAND>(outputs);
register_output<output_t::X11>(outputs);
register_output<output_t::NCURSES>(outputs);
register_output<output_t::FILE>(outputs);
register_output<output_t::HTTP>(outputs);
register_output<output_t::X11>(outputs);
register_output<output_t::WAYLAND>(outputs);
register_output<output_t::FILE>(outputs);
register_output<output_t::CONSOLE>(outputs); // global fallback - always works

for (auto out : outputs) { NORM_ERR("FOUND: %s", out->name.c_str()); }

// Sort display outputs by descending priority, to try graphical ones first.
sort(outputs.begin(), outputs.end(), &display_output_base::priority_compare);

int graphical_count = 0;

for (auto output : outputs) {
if (output->priority < 0) continue;
Copy link
Collaborator

@Caellian Caellian Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely removed priority and made ordering depend on registration order. This achieves the same result, but makes the code a lot easier to read/navigate as opposed to having this number used by a single (avoidable) sort scattered across half a dozen files. It also makes the ordering much more apparent and explicit.

Made registration order explicit, this keeps output ordering in a single file
and easier to keep track of.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian force-pushed the wbarraco/push-qwvqxqqzrkmq branch from 2190ae7 to d696b33 Compare December 7, 2024 19:05
@Caellian Caellian merged commit 2b7e4b2 into brndnmtthws:main Dec 7, 2024
38 checks passed
@stacyharper
Copy link
Contributor Author

@Caellian we still have to revert commit 940a365ada86cbf565381898e57f78c0f0a219ee to finally fix Wayland on layer shell output.

@Caellian
Copy link
Collaborator

Caellian commented Dec 7, 2024

That does make conky appear for me on hyprland. Reverted here: #2104

@stacyharper
Copy link
Contributor Author

btw @Caellian: own_window seems to still give a blank window surface to me. Anything else is borked, or have I just miss-configured conky?

@Caellian
Copy link
Collaborator

Caellian commented Dec 8, 2024

own_window doesn't do anything AFAICT, only enables an empty if statement. It will switch between layer shell and XDG shell at some point, but not now.

Send your config and WM so I can compare, my config is:

-- Conky, a system monitor https://github.com/brndnmtthws/conky
--
-- This configuration file is Lua code. You can write code in here, and it will
-- execute when Conky loads. You can use it to generate your own advanced
-- configurations.
--
-- Try this (remove the `--`):
--
--   print("Loading Conky config")
--
-- For more on Lua, see:
-- https://www.lua.org/pil/contents.html
--
-- Conky Lua API: https://conky.cc/lua

-- Configuration settings: https://conky.cc/config_settings
conky.config = {
    alignment = 'top_left',
    background = false,
    border_width = 1,
    cpu_avg_samples = 2,
    default_color = 'white',
    default_outline_color = 'white',
    default_shade_color = 'white',
    double_buffer = true,
    draw_borders = false,
    draw_graph_borders = true,
    draw_outline = false,
    draw_shades = false,
    extra_newline = false,
    font = 'DejaVu Sans Mono:size=12',
    gap_x = 60,
    gap_y = 60,
    minimum_height = 5,
    minimum_width = 5,
    net_avg_samples = 2,
    no_buffers = true,
    out_to_wayland = true,
    own_window = false, -- true,
    own_window_class = 'Conky',
    own_window_type = 'normal',
    own_window_hints = 'undecorated,sticky,below,skip_taskbar,skip_pager',
    own_window_transparent = true,
    own_window_argb_visual = true,
    show_graph_range = false,
    show_graph_scale = false,
    stippled_borders = 0,
    update_interval = 1.0,
    uppercase = false,
    use_spacer = 'none',
    use_xft = true,
    color0 = "ff0000",
}

-- Variables: https://conky.cc/variables
conky.text = [[
${color grey}Info:$color ${scroll 32 Conky $conky_version - $sysname $nodename $kernel $machine}
$hr
${color grey}Uptime:$color $uptime
${color grey}Frequency (in MHz):$color $freq
${color grey}Frequency (in GHz):$color $freq_g
${color grey}RAM Usage:$color $mem/$memmax - $memperc% ${membar 4}
${color grey}Swap Usage:$color $swap/$swapmax - $swapperc% ${swapbar 4}
${color grey}CPU Usage:$color $cpu% $color0 ${cpubar 4} $color
${color grey}Processes:$color $processes  ${color grey}Running:$color $running_processes
$hr
${color grey}File systems:
 / $color${fs_used /}/${fs_size /} ${fs_bar 6 /}
${color grey}Networking:
Up:$color ${upspeed} ${color grey} - Down:$color ${downspeed}
$hr
${color grey}Name              PID     CPU%   MEM%
${color lightgrey} ${top name 1} ${top pid 1} ${top cpu 1} ${top mem 1}
${color lightgrey} ${top name 2} ${top pid 2} ${top cpu 2} ${top mem 2}
${color lightgrey} ${top name 3} ${top pid 3} ${top cpu 3} ${top mem 3}
${color lightgrey} ${top name 4} ${top pid 4} ${top cpu 4} ${top mem 4}
]]

@stacyharper
Copy link
Contributor Author

Right, that was I thought looking at the code. Just wanna confirm :) Thanks for all you help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug related to incorrect existing implementation of some functionality display: console related to console backend display: file related to file backend display: http related to HTTP backend display: ncurses related to ncurses backend display: wayland related to Wayland backend display: x11 related to X11 backend sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants