964 dpi change #1346
964 dpi change #1346
Conversation
|
Thanks for the PR! This should fix #964 on macos and handle all DPI change events, right? So once they're supported on X11 all we'd have to do is just update to the latest glutin and we're ready to go? |
src/window.rs
Outdated
| @@ -209,7 +209,6 @@ impl Window { | |||
| .with_visibility(false) | |||
| .with_transparency(true) | |||
| .with_decorations(window_config.decorations()); | |||
| let window_builder = Window::platform_builder_ext(window_builder); | |||
There was a problem hiding this comment.
This looks like a goof when rebasing
There was a problem hiding this comment.
I had actually confused that with the function you mentioned in #964 (comment), which looks like it's already removed (presumably by you). I've removed the offending commit.
|
@chrisduerr Yes that's my understanding. |
src/display.rs
Outdated
| @@ -261,17 +267,33 @@ impl Display { | |||
| self.size_info.cell_height = ((metrics.line_height + f64::from(config.font().offset().y)) as f32).floor(); | |||
| } | |||
|
|
|||
| pub fn replace_glyph_cache(&mut self, config: &Config) -> Result<(), Error> { | |||
| let (glyph_cache, cell_width, cell_height) = | |||
| Self::new_glyph_cache(&self.window, &mut self.renderer, config)?; | |||
There was a problem hiding this comment.
Instead of creating a new glyph cache, it should be possible to alter the update_glyph_cache method, right? That might be a more efficient way to do things.
There was a problem hiding this comment.
I tried that initially, but I couldn't get it to work, though I can't now remember why exactly. I also figured since this is generally only going to happen when you move a window between monitors, the efficiency isn't exactly critical. I'm not in a position to do any more work on this for a little while, but I can revisit this later if it's a blocker.
There was a problem hiding this comment.
I can take a look at it just to make it compile on the weekend, however I'm in no position to actually test it since I only have linux machines.
Generally I think it should be possible to just update the glyph cache since we already do that for font size changes and DPI changes are basically nothing else.
Saying that this "shouldn't be critical" is probably an easy way to get bitten by it in the future. If there's an option to make something faster I think, following the general goal of the project, that option should be taken.
Thanks for all the work though, I'll report back if I managed to get it to work or if I forgot to do it over the weekend. :D
There was a problem hiding this comment.
Sounds good!
iirc, i got it to compile using update_glyph_cache, it just wouldn't do anything when switching monitors. I'm on a break before I start a new job and am without multiple monitors to test this on until I get a new work laptop. I might be able to at least test whatever you have next weekend.
There was a problem hiding this comment.
It should possible for me to test this in combination with the PR for disabling DPI scaling, I'll test it in there and then add a solution to that PR if I have one.
Then you would be able to rebase on top of that.
There was a problem hiding this comment.
hey, I think I should be able to work on this, or test something if you have it on a branch.
This makes it possible to disable DPI scaling completely, instead the the display pixel ration will always be fixed to 1.0. By default nothing has changed and DPI is still enabled, this just seems like a better way than running `WINIT_HIDPI_FACTOR=1.0 alacritty` every time the user wants to start alacritty. It would be possible to allow specifying any DPR, however I've decided against this since I'd assume it's a very rare usecase. It's also still possible to make use of `WINIT_HIDPI_FACTOR` to do this on X11. Currently this is not updated at runtime using the live config update, there is not really much of a technical limitation why this woudn't be possible, however a solution for that issue should be first added in alacritty#1346, once a system is established for changing DPI at runtime, porting that functionality to this PR should be simple.
This makes it possible to disable DPI scaling completely, instead the the display pixel ration will always be fixed to 1.0. By default nothing has changed and DPI is still enabled, this just seems like a better way than running `WINIT_HIDPI_FACTOR=1.0 alacritty` every time the user wants to start alacritty. It would be possible to allow specifying any DPR, however I've decided against this since I'd assume it's a very rare usecase. It's also still possible to make use of `WINIT_HIDPI_FACTOR` to do this on X11. Currently this is not updated at runtime using the live config update, there is not really much of a technical limitation why this woudn't be possible, however a solution for that issue should be first added in #1346, once a system is established for changing DPI at runtime, porting that functionality to this PR should be simple.
sodiumjoe
force-pushed
the
964-dpi-change
branch
from
08ba806
to
3a199db
Compare
This makes it possible to disable DPI scaling completely, instead the the display pixel ration will always be fixed to 1.0. By default nothing has changed and DPI is still enabled, this just seems like a better way than running `WINIT_HIDPI_FACTOR=1.0 alacritty` every time the user wants to start alacritty. It would be possible to allow specifying any DPR, however I've decided against this since I'd assume it's a very rare usecase. It's also still possible to make use of `WINIT_HIDPI_FACTOR` to do this on X11. Currently this is not updated at runtime using the live config update, there is not really much of a technical limitation why this woudn't be possible, however a solution for that issue should be first added in alacritty#1346, once a system is established for changing DPI at runtime, porting that functionality to this PR should be simple.
|
@sodiumjoe I'm pretty sure I've implemented everything done in this PR in the changes introduced in #591. I'd like to know if this is actually the case or if there is a difference in behavior, so I'd very much appreciate your review. If there aren't any problems, this PR should probably be closed in favor of the gultin update one. It's a bit unfortunate that there was some work duplication here, but it's still very much appreciated! |
|
@chrisduerr no worries about duplication. I'm happy to test whatever, but I'm not exactly sure which commit I should be testing? |
|
@chrisduerr if you're talking about #1403 I just tested it, and dpi change between windows appears to work great on mac 🎉 |
|
Yeah, that's what I'm talking about. Then I'd move forward with that PR. Closing this one to keep things clean. |
* Allow disabling DPI scaling This makes it possible to disable DPI scaling completely, instead the the display pixel ration will always be fixed to 1.0. By default nothing has changed and DPI is still enabled, this just seems like a better way than running `WINIT_HIDPI_FACTOR=1.0 alacritty` every time the user wants to start alacritty. It would be possible to allow specifying any DPR, however I've decided against this since I'd assume it's a very rare usecase. It's also still possible to make use of `WINIT_HIDPI_FACTOR` to do this on X11. Currently this is not updated at runtime using the live config update, there is not really much of a technical limitation why this woudn't be possible, however a solution for that issue should be first added in #1346, once a system is established for changing DPI at runtime, porting that functionality to this PR should be simple. * Add working --class and --title CLI parameters * Reduce Increase-/DecreaseFontSize step to 0.5 Until now the Increase-/DecreaseFontSize keybinds hand a step size of 1.0. Since the font size however is multiplied by two to allow more granular font size control, this lead to the bindings skipping one font size (incrementing/decrementing by +-2). To fix this the step size of the Increase-/DecreaseFontSize bindings has been reduced to the minimum step size that exists with the current font configuration (0.5). This should allow users to increment and decrement the font size by a single point instead of two. This also adds a few tests to make sure the methods for increasing/decreasing/resetting font size work properly. * Add Copy/Cut/Paste keys This just adds support for the Copy/Cut/Paste keys and sets up Copy/Paste as alternative defaults for Ctrl+Shift+C/V. * Move to cargo clippy Using clippy as a library has been deprecated, instead the `cargo clippy` command should be used instead. To comply with this change clippy has been removed from the `Cargo.toml` and is now installed with cargo when building in CI. This has also lead to a few new clippy issues to show up, this includes everything in the `font` subdirectory. This has been fixed and `font` should now be covered by clippy CI too. This also upgrades all dependencies, as a result this fixes #1341 and this fixes #1344. * Override dynamic_title when --title is specified * Change green implementation to use the macro * Ignore mouse input if window is unfocused * Make compilation of binary a phony target * Add opensuse zypper install method to readme * Fix clippy issues * Update manpage to document all CLI options The introduction of `--class` has added a flag to the CLI without adding it to the manpage. This has been fixed by updating the manpage. This also adds the default values of `--class` and `--title` to the CLI options. * Remove unnecessary clippy lint annotations We moved to "cargo clippy" in 5ba34d4 and removing the clippy lint annotations in `src/lib.rs` does not cause any additional warnings. This also changes `cargo clippy` to use the flags required for checking integration tests. * Enable clippy in font/copypasta crates Enabled clippy in the sub-crates font and copypasta. All issues that were discovered by this change have also been fixed. * Remove outdated comment about NixOS * Replace debug asserts with static_assertions To check that transmutes will work correctly without having to rely on error-prone runtime checking, the `static_assertions` crate has been introduced. This allows comparing the size of types at compile time, preventing potentially silent breakage. This fixes #1417. * Add `cargo deb` build instructions Updated the `Cargo.toml` file and added a `package.metadata.deb` subsection to define how to build a debian "deb" install file using `cargo deb`. This will allow debian/ubuntu users to install `alacritty` using their system's package manager. It also will make it easier to provide pre-built binaries for those systems. Also fixed a stray debug line in the bash autocomplete script that was writting to a tempfile. * Add config for unfocused window cursor change * Add support for cursor shape escape sequence * Add bright foreground color option It was requested in #825 that it should be possible to add an optional bright foreground color. This is now added to the primary colors structure and allows the user to set a foreground color for bold normal text. This has no effect unless the draw_bold_text_with_bright_colors option is also enabled. If the color is not specified, the bright foreground color will fall back to the normal foreground color. This fixes #825. * Fix clone URL in deb install instructions * Fix 'cargo-deb' desktop file name * Remove redundant dependency from deb build * Switch from deprecated `std::env::home_dir` to `dirs::home_dir` * Allow specifying modifiers for mouse bindings * Send newline with NumpadEnter * Add support for LCD-V pixel mode * Add binding action for hiding the window * Switch to rustup clippy component * Add optional dim foreground color Add optional color for the dim foreground (`\e[2m;`) Defaults to 2/3 of the foreground color. (same as other colors). If a bright color is dimmed, it's displayed as the normal color. The exception for this is when the bright foreground is dimmed when no bright foreground color is set. In that case it's treated as a normal foreground color and dimmed to DimForeground. To minimize the surprise for the user, the bright and dim colors have been completely removed from the default configuration file. Some documentation has also been added to make it clear to users what these options can be used for. This fixes #1448. * Fix clippy lints and run font tests on travis This fixes some existing clippy issues and runs the `font` tests through travis. Testing of copypasta crate was omitted due to problens when running on headless travis-ci environment (x11 clipboard would fail). * Ignore errors when logger can't write to output The (e)print macro will panic when there is no output available to write to, however in our scenario where we only log user errors to stderr, the better choice would be to ignore when writing to stdout or stderr is not possible. This changes the (e)print macro to make use of `write` and ignore any potential errors. Since (e)println rely on (e)print, this also solves potential failuers when calling (e)println. With this change implemented, all of logging, (e)println and (e)print should never fail even if the stdout/stderr is not available.
Resolves #964.
Resolves #71.