Feature: Add Window Padding Support

[seanstrom]

What kind of change does this PR introduce?

This change introduces a feature for adding window padding in Neovide.

Did this PR introduce a breaking change?

This change should not have introduced any breaking changes, the default window padding is zero for all sides.

Related: #140

[seanstrom]

Extra Thoughts

• I’m not sure how to expose this functionality, should it be a CLI argument or a config file setting?
• Does Neovide allow for configuration through a user config file?

[MultisampledNight]

Awesome. I'm not sure about the way the padding details are passed around, but that's okay.

[MultisampledNight]

Extra Thoughts

* I’m not sure how to expose this functionality, should it be a CLI argument or a config file setting?

* Does Neovide allow for configuration through a user config file?

I'd say a config file setting accessible from NeoVim, someone might want to change it on-the-fly. Nope, there's no dedicated Neovide config file (yet), there was some effort in #1119 but it has stalled atm.

[seanstrom]

@MultisampledNight thanks for the feedback!

I ended up querying the settings inside of Renderer::new as suggested, and then I made the window_padding field public since I need to reference it when resizing the character grid atm.

Hope that’s okay 👍

[seanstrom]

I’ll take a look at #1119 and see if I can help, would you like that to merge before this?

[MultisampledNight]

I don't have any preference about the specific order.

[MultisampledNight]

Looks pretty awesome! I just tested this and found two slight issues:

• Changing the variables of the padding at runtime has no effect. Probably because the setting is queried once at startup and then only the cached value is used, never querying the renderer settings again.
• The naming is the other way from what I'd expected. Other split settings like g:neovide_floating_blur_amount_{x,y} have the variation at the end, not at the start. But that's not too bad.

[seanstrom]

Thanks for the feedback 😸

I just updated the PR to handle redrawing the window when configuring the window padding at runtime, and I changed the window padding names based on your recommendation.

Let me know if there's anything else I should take a look at 🙏

[MultisampledNight]

Sorry for the delay, other things interfered.

[MultisampledNight]

When testing, it feels like padding changes are applied exactly one "environment change" such as a redraw too late. I believe this whole block needs to be moved right after you construct WindowPadding in order to mitigate this, a short random experiment seems to confirm this.

[seanstrom]

Hey thanks for testing this out 🙏
Can you clarify something for me, are you recommending I move this code:

let padding_changed = window_padding != self.renderer.window_padding;
if padding_changed {
    self.renderer.handle_window_padding_update(window_padding);
}

if self.saved_inner_size != new_size || font_changed || padding_changed {
    self.saved_inner_size = new_size;
    self.handle_new_grid_size(new_size);
    self.skia_renderer.resize(&self.windowed_context);
}

Above this code?

if REDRAW_SCHEDULER.should_draw() || SETTINGS.get::<WindowSettings>().no_idle {
    font_changed = self.renderer.draw_frame(self.skia_renderer.canvas(), dt);
    self.skia_renderer.gr_context.flush(None);
    self.windowed_context.swap_buffers().unwrap();
}

[seanstrom]

Side question, are you referring to the visual behavior that happens during a resize or when the padding changes? I've experimented with moving the construction window padding like so:

let window_settings = SETTINGS.get::<WindowSettings>();
let window_padding = WindowPadding {
    top: window_settings.padding_top,
    left: window_settings.padding_left,
    right: window_settings.padding_right,
    bottom: window_settings.padding_bottom,
};

let padding_changed = window_padding != self.renderer.window_padding;
if padding_changed {
    self.renderer.window_padding = window_padding;
}

if self.saved_inner_size != new_size || font_changed || padding_changed {
    self.saved_inner_size = new_size;
    self.handle_new_grid_size(new_size);
    self.skia_renderer.resize(&self.windowed_context);
}

Which could resolve some issues with accidentally using an outdated window padding, and seemed a bit snappier when resizing, but let me know if this is related to what you were saying.

[MultisampledNight]

Yup, that's exactly what I meant. Thank you!

[seanstrom]

Okay great! I just pushed up some commits that implement your recommendations. Let me know if this seems good 👍

[MultisampledNight]

Otherwise your work is awesome! I believe those things should be the last ones.

[MultisampledNight]

(for what it's worth please do drop by a short notice if you think you're done, slightly unsure atm)

[seanstrom]

Yup I think I’m done making changes. Though please double check my latest changes please, and then I think it’s ready to merge.

[MultisampledNight]

Yeah, I miscommunicated on my part about how I meant to reorder the respect-resize hunk. The change is simple enough, I'll do it real quick.

[seanstrom]

Okay sounds good, thank you for the assist 🙌

[MultisampledNight]

"real quick" is what it said and came back half an hour later. Anyways, good to go I guess! chooooooooooo chooooooooooooooooo thank you!