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

feat: scale inner and outer gaps by monitor DPI #756

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

JonasWischeropp
Copy link
Contributor

I was trying to make my zebar config work with multiple monitors that have different display scaling factors.
Related discussion: glzr-io/zebar#72
This PR suggests a possible fix by scaling outer_gap and inner_gap by the display scale.
Let me know if I should change something.

Err(_) => 1_f32,
},
};
self.0.borrow().inner_gap.clone() * scale
Copy link
Member

Choose a reason for hiding this comment

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

like in the Zebar PR, think we should only be scaling pixels here

would suggest adding a new field:

#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(rename_all(serialize = "camelCase"))]
pub struct GapsConfig {
  /// Whether to scale the gaps with the DPI of the monitor.
  #[serde(default = "default_bool::<true>")]
  pub scale_with_dpi: bool,
  
// sample-config.yaml:
// gaps:
//   # Whether to scale the gaps with the DPI of the monitor.
//   scale_with_dpi: true

that way, we can make it more explicit that this gets scaled, since all other px values in the config deal with physical pixels.

would then have to change TilingSizeGetters::inner_gap, TilingSizeGetters::set_inner_gap, Workspace::set_outer_gap to instead get/set the GapsConfig struct.

to deal with only scaling px values and not %, maybe we could add something like LengthValue::to_px_scaled(&self, total_px: i32, scale_factor: f32)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@JonasWischeropp JonasWischeropp marked this pull request as draft October 2, 2024 14:14
@lars-berger lars-berger marked this pull request as ready for review October 4, 2024 06:06
@lars-berger
Copy link
Member

Will merge as is and add the gaps.scale_with_dpi config option in a follow-up PR 👍

@lars-berger lars-berger changed the title feat: scale gaps by display scale feat: scale inner and outer gaps by monitor DPI Oct 4, 2024
@lars-berger lars-berger merged commit adc6d56 into glzr-io:main Oct 4, 2024
2 checks passed
@JonasWischeropp
Copy link
Contributor Author

Ok, thank you.

Copy link

github-actions bot commented Oct 4, 2024

🎉 This PR is included in version 3.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants