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

Remove dpi scaling of maximum width #1877

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Remove dpi scaling of maximum width #1877

merged 5 commits into from
Apr 30, 2024

Conversation

Caellian
Copy link
Collaborator

@Caellian Caellian commented Apr 30, 2024

This PR removes dpi_scaleing of maximum width setting.

This seems incorrect because I expect maximum width to use absolute pixel value, otherwise meaning of maximum_width setting changes with DPI:

image
NOTE: Height difference is caused by font height, conky height can't be limited.

This fixes #1528.

This PR also fixes a regression introduced in #1841, where I incorrectly assumed templated base class function would be overriden by display outputs so DPI scaling was completely disabled.

Other changes

This PR also:

  • sets workspace dimensions from WL output_geometry because workspace is used for some calculations in conky.cc.
    • this requires more work for proper support as noted in comments, but it should make Wayland less broken.
  • adds conky xdg dir and script parent directory to Lua path so that scripts don't have to update it manually.

Testing

Set workspace dimensions from WL output_geometry

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Copy link

netlify bot commented Apr 30, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit 6d194af
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/66314effc9a3a600082b3523

@github-actions github-actions bot added sources PR modifies project sources display: x11 related to X11 backend display: wayland related to Wayland backend labels Apr 30, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian changed the title Remove dpi scaling maximum width Remove dpi scaling of maximum width Apr 30, 2024
@Caellian Caellian marked this pull request as ready for review April 30, 2024 20:06
Comment on lines +1965 to +1997
// Extend lua package.path so scripts can use relative paths
{
struct stat file_stat {};

std::string path_ext;

// add XDG directory to lua path
auto xdg_path =
std::filesystem::path(to_real_path(XDG_CONFIG_FILE)).parent_path();
if (stat(xdg_path.c_str(), &file_stat) == 0) {
path_ext.push_back(';');
path_ext.append(xdg_path);
path_ext.append("/?.lua");
}

auto parent_path = current_config.parent_path();
if (xdg_path != parent_path && stat(path_ext.c_str(), &file_stat) == 0) {
path_ext.push_back(';');
path_ext.append(parent_path);
path_ext.append("/?.lua");
}

l.getglobal("package");
l.getfield(-1, "path");

auto path = l.tostring(-1);
path.append(path_ext);
l.pop();
l.pushstring(path.c_str());

l.setfield(-2, "path");
l.pop();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding $HOME/.config/conky and config parent directory to lua path before loading the script.

if (use_xft.get(*state) && xft_dpi > 0) {
return (value * xft_dpi + (value > 0 ? 48 : -48)) / PIXELS_PER_INCH;
Copy link
Collaborator Author

@Caellian Caellian Apr 30, 2024

Choose a reason for hiding this comment

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

I believe (value > 0 ? 48 : -48) was rounding the result for int values before, so I replaced it with ceil and floor in new code. PIXELS_PER_INCH is 96.

Comment on lines +304 to +312
// TODO: Add support for proper output management through:
// - xdg-output-unstable-v1
// Maybe also support (if XDG protocol not reported):
// - kde-output-management(-v2)
// - wlr-output-management-unstable-v1
workarea[0] = x; // TODO: use xdg_output.logical_position
workarea[1] = y;
workarea[2] = physical_width;
workarea[3] = physical_height;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the workarea change.

Copy link
Owner

@brndnmtthws brndnmtthws left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@@ -275,7 +276,7 @@ int text_width = 1,
struct information info;

/* path to config file */
std::string current_config;
std::filesystem::path current_config;
Copy link
Owner

Choose a reason for hiding this comment

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

Nice

@brndnmtthws brndnmtthws added the bug related to incorrect existing implementation of some functionality label Apr 30, 2024
@Caellian Caellian merged commit e1f3013 into main Apr 30, 2024
62 checks passed
@Caellian Caellian deleted the fix/dpi-scaling branch April 30, 2024 20:21
@Caellian Caellian added the rendering related to code that handles rendering, excluding the used graphics API label Apr 30, 2024
Caellian added a commit that referenced this pull request May 18, 2024
This was left over from #1877.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Caellian added a commit that referenced this pull request May 22, 2024
Caellian added a commit that referenced this pull request May 30, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Caellian added a commit that referenced this pull request Jun 1, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
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: wayland related to Wayland backend display: x11 related to X11 backend rendering related to code that handles rendering, excluding the used graphics API sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: use_xft = true breaks window sizing
2 participants