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]: conky pixel unit isn't scaled by screen DPI #1925

Closed
LinuxOnTheDesktop opened this issue May 18, 2024 · 25 comments
Closed

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

LinuxOnTheDesktop opened this issue May 18, 2024 · 25 comments
Labels
priority: high A high priority issue or PR rendering Issue or PR related to rendering

Comments

@LinuxOnTheDesktop
Copy link

LinuxOnTheDesktop commented May 18, 2024

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 LinuxOnTheDesktop added bug Bug report or bug fix PR triage Issue that hasn't been verified labels May 18, 2024
@LinuxOnTheDesktop
Copy link
Author

LinuxOnTheDesktop commented May 18, 2024

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
Copy link
Collaborator

Caellian commented May 18, 2024

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 Caellian added wontfix Won't be fixed: not a conky bug, unreasonable effort or can't fix due to external limitations rendering Issue or PR related to rendering and removed bug Bug report or bug fix PR triage Issue that hasn't been verified labels May 18, 2024
@Caellian
Copy link
Collaborator

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).

@Caellian Caellian closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2024
@LinuxOnTheDesktop
Copy link
Author

@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
Copy link
Collaborator

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
Copy link
Author

My mistake. Apologies.

@zygfryd
Copy link

zygfryd commented May 19, 2024

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
Copy link

zygfryd commented May 19, 2024

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 Caellian reopened this May 24, 2024
@Caellian Caellian changed the title [Bug]: latest release truncates Conky on a HiDPI system [Bug]: conky pixel unit isn't scaled by screen DPI May 24, 2024
@Caellian
Copy link
Collaborator

@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
Copy link

zygfryd commented May 25, 2024

@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 Caellian added priority: high A high priority issue or PR and removed wontfix Won't be fixed: not a conky bug, unreasonable effort or can't fix due to external limitations labels May 25, 2024
@Caellian
Copy link
Collaborator

Caellian commented May 26, 2024

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
Copy link
Collaborator

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
Copy link

zygfryd commented May 30, 2024

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
Copy link
Collaborator

Caellian commented Jun 1, 2024

#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
Copy link

zygfryd commented Jun 1, 2024

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
Copy link
Collaborator

Caellian commented Jun 1, 2024

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

Yes, pushed now.

@zygfryd
Copy link

zygfryd commented Jun 2, 2024

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
Copy link
Contributor

agorgl commented Jun 2, 2024

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
Copy link
Collaborator

Caellian commented Jun 5, 2024

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
Copy link

zygfryd commented Jun 5, 2024

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
Copy link
Collaborator

Caellian commented Jun 7, 2024

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
Copy link
Contributor

agorgl commented Jul 2, 2024

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
Copy link
Collaborator

Caellian commented Jul 2, 2024

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
Copy link
Contributor

agorgl commented Jul 2, 2024

It seems that there is an inconsistency between minimum_width

text_size = conky::vec2i(dpi_scale(minimum_width.get(*state)), 0);
and minimum_height
text_size = text_size.max(conky::vec2i(text_size.x() + 1, minimum_height.get(*state)));

@agorgl
Copy link
Contributor

agorgl commented Jul 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high A high priority issue or PR rendering Issue or PR related to rendering
Projects
None yet
Development

No branches or pull requests

4 participants