brndnmtthws / conky

Light-weight system monitor for X, Wayland (sort of), and other things, too
https://conky.cc
GNU General Public License v3.0
7.17k stars 615 forks source link

[Bug]: conky pixel unit isn't scaled by screen DPI #1925

Closed LinuxOnTheDesktop closed 3 months ago

LinuxOnTheDesktop commented 4 months ago

What happened?

I built the latest release and installed it on two computers. On one computer, which has DPI in no sense, all was fine. On the other computer, which has a HiDPI screen and DPI scaling set to times-two, the right-hand half of my Conky is missing. In the previous GitHub release I did not have the problem.

Screenshot:

image

I have Linux Mint 21.3 Cinnamon.

Version

1.21.1-pre-256ffdb7

Which OS/distro are you seeing the problem on?

Linux (other)

Conky config

home = os.getenv("HOME")
dofile ( home .. '/<redacted>/Conky/config/config_x1.lua' )
conky.config = configuration

-- Conky objects: http://conky.sourceforge.net/variables.html
-- Cannot without fuckery remove border from bars (as against from graphs).

-- To monitor cpu usage: top -p "$(pgrep conky)"

conky.text = [[
${offset 10}\
# -----------------------
# CPU usage and frequency
# -----------------------
# CPU
${color4}\
cpu\
# CPU usage
${offset 12}\
${color1}\
${cpubar cpu0 8,25}\
# Frequency
${offset 12}\
${color #6A786A}\
${freq_g}\
${offset 2}GHz\
#
# -----------------------------------------------------------------
# TEMPERATURE
# CPU (the mean of all cores).
# Seems to be no other useful sensor (only wifi & individual cores).
# Use monospace font.
# CPU critical temperature (according to Intel): 100 degrees.
# ------------------------------------------------------------------
#
# CPU TEMP
${offset 12}\
${if_match ${acpitemp}<66}\
# Pastel green
${color1}\
${else}\
${if_match ${acpitemp}<76}\
# Pastel yellow
${color2}\
${else}\
${if_match ${acpitemp}<86}\
# Orange
${color orange}\
${else}\
${if_match ${acpitemp}<99}\
# Pastel red
${color3}\
${else}\
# Red
${color red}\
${endif}\
${endif}\
${endif}\
${endif}\
${acpitemp}\
# Temperature symbol
${offset 1}${font Hack:italic:bold:size=10}${color8}°${font}\
# ------
# DRIVES
# ------
#
${goto 192}\
# ROOT
${font Hack:bold:size=9}${color4}\
/\
${font}${offset 12}\
${if_match ${fs_used_perc /} > 80}\
${color3}\
${else}\
${if_match ${fs_used_perc /} > 75}\
${color2}\
${else}\
${if_match ${fs_used_perc /} > 65}\
${color6}\
${else}\
${color1}\
${endif}\
${endif}\
${endif}\
${fs_used_perc /}%\
# HOME
${goto 258}\
${font Hack:bold:size=9}${color4}\
~\
${font}${offset 8}\
${if_match ${fs_used_perc /home} > 90}\
${color3}\
${else}\
${if_match ${fs_used_perc /home} > 85}\
${color2}\
${else}\
${if_match ${fs_used_perc /home} > 69}\
${color6}\
${else}\
${color1}\
${endif}\
${endif}\
${endif}\
${fs_used_perc /home}%\
${font}\
# RAM
${goto 322}\
${color4}\
r\
${offset 8}\
${if_match ${memperc} > 90}\
${color3}\
${else}\
${if_match ${memperc} > 65}\
${color2}\
${else}\
${color1}\
${endif}\
${endif}\
${memperc}%\
${font}\
# SWAP
${goto 390}\
${color4}\
s\
${offset 8}\
${if_match ${swapperc} > 80}\
${color3}\
${else}${if_match ${swapperc} > 64}\
${color2}\
${else}${if_match ${swapperc} > 50}\
${color6}\
${else}\
${color1}\
${endif}${endif}${endif}\
${swapperc /swap}%\
${font}\
# --------
# NETWORK
# --------
${goto 458}\
${if_up enp0s31f6}\
# --------
# Ethernet
# --------
${lua_parse netIsUp}\
${color1}Ethernet${offset 12}${color7}${downspeedgraph enp0s31f6 12,18 93CC98 B6F51F 9765KiB -t}\
${offset 4}${upspeedgraph enp0s31f6 12,18 93CC98 B6F51F 9765KiB -t}\
${color}\
${offset 16}${lua_parse vpn}\
${offset 16}${lua_parse firewall}\
${else}\
# ----
# Wifi
# ----
${if_up wlp2s0}\
${lua_parse netIsUp}\
${if_match ${wireless_link_qual_perc wlp2s0}>69}\
${color5}\
${else}\
${if_match ${wireless_link_qual_perc wlp2s0}>49}\
${color6}\
${else}\
${if_match ${wireless_link_qual_perc wlp2s0}>29}\
${color2}\
${else}\
${color3}\
${endif}\
${endif}\
${endif}\
${execi 3.5 /home/<redacted>/scripts/conky/c/wifiName_x1_execi}\
${offset 12}${color7}${downspeedgraph wlp2s0 13,18 93CC98 B6F51F 9765KiB -t}\
${offset 4}${upspeedgraph wlp2s0 13,18 93CC98 B6F51F 9765KiB -t}\
${color}\
${offset 16}${lua_parse vpn}\
${offset 16}${lua_parse firewall}\
${else}\
${if_up bnep0}\
# ----
# Cell
# ----
${lua_parse netIsUp}\
${color1}CELL${offset 12}${color7}${downspeedgraph wlp2s0 13,18 93CC98 B6F51F 9765KiB -t}\
${offset 4}${upspeedgraph wlp2s0 13,18 93CC98 B6F51F 9765KiB -t}\
${color}\
${offset 16}${lua_parse vpn}\
${offset 16}${lua_parse firewall}\
${else}\
# ----
# USB
# ----
${if_up enp0s20f0u1}\
${offset 5}${color1}USB${offset 25}${color7}${downspeedgraph enp0s31f6 12,18 93CC98 B6F51F 9765KiB -t}\
${offset 5}${upspeedgraph enp0s31f6 12,18 93CC98 B6F51F 9765KiB -t}\
${color}\
${offset 16}${lua_parse vpn}\
${offset 16}${lua_parse firewall}\
${else}\
${color4}\
# -------
# Nothing
# -------
# The point of using lua here is to have a way of preventing *immediate* text display.
${lua_parse netIsDown}\
${endif}\
${endif}\
${endif}\
${endif}\
#
# ------
# POWER
# Seems - don't know why - we need to reset the font.
# ------
${font}\
${offset 26}\
${if_match "${battery}"=="charged"}\
# Charged
${color5}\
+++\
${font}\
${else}\
# Either charging or discharging.
# NB: Script below sets colour.
# Conky's inbuilt battery support is inadequate.
${lua_parse battery}\
${endif}\
${color}\
# --------------------------------------------------------
# BRIGHTNESS & Redshift
# Use monospace font, together with a spacing-aware script.
# ---------------------------------------------------------
${goto 784}\
${voffset -3}\
${font Symbola:size=12}\
⛯\
${font}\
${offset 5}\
# Set the colour..
${lua_parse redshift}\
# Now print brightness value. NB: Conky is about to add native functionality for this!
${lua_parse brightness}\
# Using lua as versus C saves 15% of CPU!
${color}\
${font}\
${voffset -3}\
# ---------------
# VOLUME & SOURCE
# ---------------
${goto 862}\
${font Symbola:size=12:bold}\
${head ~/tmp/conkyRamdisk/soundSource 1 2}\
${font}\
${offset 5}\
${color1}\
# ${if_pa_sink_muted} is broken.
${exec /home/<redacted>/<redacted>/scripts/conky/c/volume_exec}\
${color}\
# --------------------------------------------------------
# DATE & TIME
# If use ${offset} here [at start?], get problems with right-aligning.
# So, use spaces, with different font size.
# For time syntax: https://linux.die.net/man/3/strftime
# --------------------------------------------------------
${goto 950}\
${font Hack:size=9.5}\
${color4}\
# This (below) is the DAY, in full (e.g. 'Monday'), followed by a space.
${time %A} \
# This (below) is:
# The DAY NUMBER of the month (e.g.' 1');
# the MONTH (in text, abbreviated and followed by ' ', then as a number in parentheses and preceded by '#',)'
# the YEAR.
${time %-d} ${time %b} (\#${time %-m}) ${time %Y}\
# TIME (as against date), in this format: 12 hour clock, H:M AM/PM
${offset 10}\
${font Hack:size=9.5:bold}\
${time %l}\
:\
${time %M}\
# Seconds
${offset 5}\
${voffset -3}\
${font Hack:size=7}\
${color #CFD1CF}\
${time %S}\
${font}\
# This (below) is the AM / PM and timezone.
# NB: a peculiarity in my locale means that cannot get AM/PM in capitals by doing ${time %p},
# but ${time %^p} works.
${offset 6}\
${time %^p}\
${offset 11}\
${color #8D8F8D}\
${time %Z}\
${color}\
# --------------------------------------------
# SECOND ROW
# Note the lack of a slash after the 'voffset'.
# --------------------------------------------
${voffset 12}
${font Hack:size=7.5}\
# PC name, bios, OS, kernel, boot time.
${goto 41}\
${color #87B587}\
${no_update ${nodename_short}}\
${color #808080} · \
${color #BEBEBE}bios \
${color #87B587}\
${no_update ${head ~/tmp/conkyRamdisk/BIOS 1 60000}}\
${color #808080} · \
${color #87B587}\
${no_update ${head ~/tmp/conkyRamdisk/OS 1 60000}}\
${color #808080} · \
${color #87B587}\
${no_update ${kernel}}\
${color #808080}\
 · \
# 'head' syntax: <file> <num lines> <update multiplier>
${color #87B587}\
${no_update ${head ~/tmp/conkyRamdisk/boot 1 20000}}\
#
# *Conky version*
${offset 30}\
${color #BEBEBE}conky ${color #87B587}${no_update ${conky_version}}\
#
# *Log*
${goto 645}\
${color #BEBEBE}log \
${lua_parse log}\
#
# MAIL
# ----
${goto 830}\
${color white}\
${lua_parse mail}\
#
# *Services*
${goto 994}\
${color #BEBEBE}\
services \
${lua_parse services}\
#
# *Syncthing*
${goto 1150}\
${color #BEBEBE}\
sync \
${lua_parse sync}\
${font}\
#
#
# --------------------------------------------
# THIRD ROW
# Note the lack of a slash after the 'voffset'.
# --------------------------------------------
${voffset 10}
${alignc}\
${color9}\
${font Hack:size=7.5}\
# LUA
${lua_parse row}\
${font}\
# This last line - any operation at all? - reduces flicker, for some reason. So does having a line of space, and no voffset, to start the row.
#
# --------------------------------------------
# FOURTH ROW
# Note the lack of a slash after the 'voffset'.
# --------------------------------------------
# MUSIC
#
${voffset 10}
${alignc}\
${font Hack:size=9}\
${execpi 5.8 /usr/bin/nice -n 3 /home/<redacted>/<redacted>/scripts/conky/c/song_execpi}\
${font}
#
# Comments of any kind after the closing brackets below causes problems.
#
# ---------------
# * * * END * * *
# ---------------
#
]]

Stack trace

No response

Relevant log output

conky_test: INFO - starting Conky ..
conky: desktop window (0x6200082) is subwindow of root window (0x776)
conky: window type - desktop
conky: drawing to created window (0x6e00002)
conky: drawing to double buffer
conky: FOUND: console
conky: FOUND: ncurses
conky: FOUND: file
conky: FOUND: x11
conky: 'cinnamon' x11 session running 'X-Cinnamon' destop [_sic_ - i.e., there is a misspelling of 'desktop' here]

EDITED.

LinuxOnTheDesktop commented 4 months ago

Ah: the problem seems to be as follows. _Between this build and the last (or perhaps the one before last) the handling of maximum_width has changed_. Seemingly, now, that variable should be set to the actual, full resolution (if one wants to use the full screen), rather than - as it was before - to the resolution as understood after DPI scaling. I presume that the same holds for minimum_height.

(The config I posted lacks those variables: I have them in a separate lua file - or a file that differs across the two PCs at issue.)

It seems to me that either the behaviour should be reverted or else users should be made aware of the change.

Caellian commented 4 months ago

It seems to me that either the behaviour should be reverted or else users should be made aware of the change.

It's impossible to make a consistent looking configs if maximum_width depends on DPI. This was cause of #1528. Given that conky uses pixel size for most things (e.g. image size), automatic scaling of only certain properties is (I believe) wrong. It would maybe make sense to add a separate global scaling factor.

1877 and #1862 fixed this partially.

I just created #1926 as I found another place where maximum_width was scaled. I also removed scaling of minimum_width.

It's a bit confusing because we're not using units like rem/em/ch, and pixel dimensions should be affected by DPI. But if we scale px, the unit should really be considered pt at that point (hah, punny). Likely the next step would be adding a unit resolver that allows configs to specify different unit values like CSS does and make the number without any units default to pt which would revert the old behavior - this would provide means to fix most inconsistencies when it comes to content sizing.

Caellian commented 4 months ago

I made the name of the PR clear (which is referenced in release notes), besides that I'm not sure how we can make it clearer that the behavior changed.

Closing this as wontfix because I don't see it as a bug and the introduced changes make conky behave correctly (more in line how other software (e.g. browsers) works).

LinuxOnTheDesktop commented 4 months ago

@Caellian

Thank you for the responses.

So, now and henceforth DPI scaling will not affect minimum_width and maximum_width? That is what your last post says, I take it. Still: your first post (here) seemed to suggest that, no, things would be put back to how they were before; but I understood that first post only poorly. (For, that post is technical.)

I'll go with the 'wontfix' interpretation (as we might call it). Given that situation, I proceed to the following.

I'm not sure how we can make it clearer that the behavior changed.

You could add a conspicuous comment to the release that made the change. That way, users like me how installed that release (which was either the most recent one or the one before that) will not get a nasty surprise.

Caellian commented 4 months ago

Release notes are generated from PRs. The bug fix PR that changed behavior did say that max width no longer uses DPI in the title, so it was in the release notes.

LinuxOnTheDesktop commented 4 months ago

My mistake. Apologies.

zygfryd commented 4 months ago

I'm using Wayland with 200% scaling on 4K monitors.

In 1.19 the scaling was consistent: Screenshot_20240519_124156

In 1.20 graphical elements are now at 100% scaling while fonts are at 200%: Screenshot_20240519_124542

I'm skeptical about this being an improvement. I guess I need to stick to 1.19 until the global scaling factor gets implemented?

zygfryd commented 4 months ago

Closing this as wontfix because I don't see it as a bug and the introduced changes make conky behave correctly (more in line how other software (e.g. browsers) works).

I was under the impression browsers changed the size of the logical pixel, just like conky used to do.

Caellian commented 4 months ago

@zygfryd You're right. The exact name for the scaling factor used is Device Pixel Ratio (dPR). Though it's almost a standard practice for websites to set <meta name="viewport" content="width=device-width, initial-scale=1.0"> which sets dPR to 1.0 and makes px respect actual pixel size. dPR is based on DPI, but it's not an exact match (adjusted for expected screen viewing distance).

This was necessary because old phones (iPhone) had to make websites smaller to fit the screen, so they reported incorrect screen dimensions to websites/browsers to achieve this effect. Logical dimensions were introduced in order to make (mostly) images behave correctly on different DPI screens.

Anywho, I was planning on adding a way of handling different units which would default to something like "logical pixels" if no unit was specified. I see how current behavior could be confusing. While I do believe px unit should really be pixels and not adjusted because it's misleading otherwise (e.g. scaled/blurry images even though exact px size is used), I explored what other UI is doing a bit more and it seems most of UI toolkits do scale the px value so I decided to reopen this issue.

zygfryd commented 4 months ago

@zygfryd You're right. The exact name for the scaling factor used is Device Pixel Ratio (dPR). Though it's almost a standard practice for websites to set <meta name="viewport" content="width=device-width, initial-scale=1.0"> which sets dPR to 1.0 and makes px respect actual pixel size.

That actually just scales your app 1:1 to the viewport in logical pixels. On a phone it'll mean you'll be working with widths in the 300-400px range, not the physical 1000px+ range. Otherwise the usual method of width breakpoints for responsive design wouldn't work.

Anywho, I was planning on adding a way of handling different units which would default to something like "logical pixels" if no unit was specified. I see how current behavior could be confusing. While I do believe px unit should really be pixels and not adjusted because it's misleading otherwise (e.g. scaled/blurry images even though exact px size is used), I explored what other UI is doing a bit more and it seems most of UI toolkits do scale the px value so I decided to reopen this issue.

Glad to see that.

From a user's perspective, it'd be ideal for a conky config to work like it used to in 1.19, that is regardless of monitor density the conky window takes the same, scaled, amount of logical space. Desktop scaling is a decision the user makes and shouldn't be worked around, but respected. If the user wants to work around it, then conky might give the option of overriding the scaling factor it obtains from X11 or Wayland. It'd be tedious to have to rewrite all the sizes in your config when moving between monitors with different densities and scaling factors.

Caellian commented 4 months ago

I started work on this, but adding proper units touches a lot of code, and I'll have to write tests. I'm mostly done, just have to update all previous pixel ints with new unit value.

Caellian commented 3 months ago

Can you try revert/dpi-changes branch and let me know whether this is fixed?

git clone -b revert/dpi-changes git@github.com:brndnmtthws/conky.git test_conky
cmake -S test_conky -B test_conky/build
cmake --build test_conky/build

# run with
./test_conky/build/src/conky <options>
zygfryd commented 3 months ago

Can you try revert/dpi-changes branch and let me know whether this is fixed?

Rendering looks good: text, bars and graphs seem to be the same size as in 1.19.8. maximum_width is unscaled (so contents are cut off using config from 1.19.8). Desktop reserved space (in panel mode) seems to be double the actual window size.

Caellian commented 3 months ago

1949 was merged and reverted removal of DPI scaling. I'll implement units as I manage, but this fixes the issue in the meantime.

maximum_width is unscaled (so contents are cut off using config from 1.19.8).

1952 was merged and fixes width growth with content.

Desktop reserved space (in panel mode) seems to be double the actual window size.

This is the only thing that's left unaddressed. @zygfryd please check out main and let me know whether the issue is still present.

If so, I created a separate branch for testing test/workaread-adjustments - panel with left alignment has dpi scale applied, and with the right alignment has the inverse applied (so it scales down).

zygfryd commented 3 months ago

Turns out I was using X11 output on Wayland this whole time. main fixes rendering/scaling issues, but reserved space issue remains. I also tested Wayland output this time and that's a whole other can of issues, not related to this one. I could only get Wayland working on my system install of 1.19.8, manually built main just didn't want to output anything.

If so, I created a separate branch for testing test/workaread-adjustments - panel with left alignment has dpi scale applied, and with the right alignment has the inverse applied (so it scales down).

Couldn't find it, forgot to push it perhaps?

Caellian commented 3 months ago

Couldn't find it, forgot to push it perhaps?

Yes, pushed now.

zygfryd commented 3 months ago

test/workaread-adjustments - panel with left alignment has dpi scale applied, and with the right alignment has the inverse applied (so it scales down).

Left alignment results in 3x reserved space (tested by maximizing a window), right alignment results in no reserved space.

agorgl commented 3 months ago

Can we do an intermediate 1.21.3 release with the dpi reverts applied in order to fix the scaling issues in the meantime?

Caellian commented 3 months ago

Left alignment results in 3x reserved space (tested by maximizing a window), right alignment results in no reserved space.

Thank you for testing it, I appreciate it. Can you just lmk what your DPI is (xrdb -query | grep -i dpi) - that should be enough info for me to figure it out when I get to this.

Can we do an intermediate 1.21.3 release with the dpi reverts applied in order to fix the scaling issues in the meantime?

I think version bump was merged, it will be released soon. You can build from source in the meantime.

zygfryd commented 3 months ago

Thank you for testing it, I appreciate it. Can you just lmk what your DPI is (xrdb -query | grep -i dpi) - that should be enough info for me to figure it out when I get to this.

192

Caellian commented 3 months ago

I'm going to close this as the issue has been solved.

@zygfryd I'm creating a separate issue with smaller scope for the problem you've detailed: #1961

agorgl commented 2 months ago

I just tested 1.21.3 and now my custom cairo lua conky widgets seem to scale properly. The minimum_height though still seem to be affected, is this something that was not part of the revert?

Caellian commented 2 months ago

Should've been. It's possible some change was missed though because I spread those changes through several commits. It's not too difficult to go through all of them since last working version except the already reverted ones and check. I'll look into it this week. I feel somewhat bad bc I caused this and haven't had time to fully fix it yet, but I promise it will be fixed (and better?) soon ™️.

agorgl commented 2 months ago

It seems that there is an inconsistency between minimum_width https://github.com/brndnmtthws/conky/blob/b0e2ec483f18113f1bb1a74bfd85b2f1fd327d12/src/conky.cc#L861 and minimum_height https://github.com/brndnmtthws/conky/blob/b0e2ec483f18113f1bb1a74bfd85b2f1fd327d12/src/conky.cc#L865

agorgl commented 2 months ago

I fixed it and tested it in the above PR. Can we bump the patch version after this fix?