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

Add shortcuts to toggle antialiasing #149

Merged
merged 7 commits into from
Aug 24, 2020

Conversation

Aloso
Copy link
Contributor

@Aloso Aloso commented Aug 22, 2020

This adds shortcuts to toggle antialiasing. The current value is stored in the cache file. There are three possible values:

  • always – turn antialiasing on
  • never – turn antialiasing off
  • auto – antialiasing depends on the zoom factor

There are two new key bindings:

  • toggle_antialias (default S): Sets antialiasing to always or never
  • automatic_antialias (default Alt+S): Sets antialiasing to auto

The cfg.toml has a new setting image.antialiasing, which can have the following values:

  • always, never, auto – see above
  • previous – restores the setting from the last session

Closes #26

@Aloso Aloso marked this pull request as ready for review August 22, 2020 17:30
pub fn toggle_antialias(&mut self) {
let mut cache = self.cache.lock().unwrap();
let aa = match cache.image.antialiasing {
Antialias::Auto if self.img_texel_size < 4f32 => Antialias::Never,
Copy link
Owner

Choose a reason for hiding this comment

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

This is cool

@@ -35,6 +37,8 @@ lazy_static! {
m.insert(PLAY_ANIM_NAME, vec!["Alt+A", "Alt+V"]);
m.insert(PLAY_PRESENT_NAME, vec!["P"]);
m.insert(PLAY_PRESENT_RND_NAME, vec!["Alt+P"]);
m.insert(TOGGLE_ANTIALIAS, vec!["S"]);
m.insert(SET_AUTOMATIC_ANTIALIAS, vec!["Alt+S"]);
Copy link
Owner

Choose a reason for hiding this comment

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

I like that this always sets to autmatic no matter what. This helps combat that sometimes the difference is barely noticable even if a changed happened in filtering.

@ArturKovacs
Copy link
Owner

Thanks you, this is nice; there are a few things I'd like to add

  • Currently this only applies to magnificaiton. I'm wondering if it would be a good idea to apply this to minification as well.
  • I think that there could be a cfg field for remembering the aa setting. For example I would like to have the aa set to auto every time I open an emulsion window. But I understand that it can be more preferabble to use the previous aa setting.
  • I like the initiative to more compactly fit the contents of the usage panel; but for me the previous layout was definitely cleaner. I'd prefer if you'd just make a page size in the odt a bit taller to accomodate the new rows. Of course if we reach a point where there are too many items to fit vertically, we'll have to find a new alternative but right now I can't think of anything that would be clean and more compact.

@Aloso
Copy link
Contributor Author

Aloso commented Aug 23, 2020

Currently this only applies to magnificaiton. I'm wondering if it would be a good idea to apply this to minification as well.

How do you mean? When pressing S, antialiasing is toggled irrespective of the zoom level. Or do you mean something different?

I think that there could be a cfg field for remembering the aa setting.

Now that I think of it, I agree that using cfg.toml would be better than cache.toml.

I like the initiative to more compactly fit the contents of the usage panel; but for me the previous layout was definitely cleaner.

I'll change it back then.

@ArturKovacs
Copy link
Owner

What I meant by minification was that when the image is shrunk we still use smoothing even if aa is off. But I honestly don't know if anyone would want to turn that off so let's just leave it as it is. If someone needs that, they'll open an issue.

Now that I think of it, I agree that using config.toml would be better than cache.toml.

To clarify, I was thinking of using both. The cache would remain the way you made it and the cfg would have an optional field called use_last_aa which is a bool which is false by default. It's also because emulsion shouldn't write to the cfg file.

@Aloso
Copy link
Contributor Author

Aloso commented Aug 24, 2020

@ArturKovacs I think I addressed all of your concerns.

src/picture_widget.rs Outdated Show resolved Hide resolved
@ArturKovacs
Copy link
Owner

Thanks!

@ArturKovacs ArturKovacs merged commit 1a84ac2 into ArturKovacs:master Aug 24, 2020
@Aloso Aloso deleted the antialiasing branch January 13, 2021 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Toggle antialiasing
2 participants