Remember windows size via pixel #1706

chomosuke · 2022-12-30T00:30:26Z

What kind of change does this PR introduce?

Fix
- closes Window size is increasing with every start of neovide #1636.
Feature
- added --size cmd option.

Did this PR introduce a breaking change?

A breaking change includes anything that breaks backwards compatibility either at compile or run time.

No

& move parse_window_geometry

A-Lamia · 2023-01-04T01:44:58Z

On first start up window opens in minimal possible size, after that everything works as intended.

Tested on windows 10.

chomosuke · 2023-01-04T01:52:00Z

On first start up window opens in minimal possible size, after that everything works as intended.

This is expected, it's because it's reading the last saved character size and interpreting that as pixel size.

A-Lamia · 2023-01-04T01:58:55Z

Then everything is perfect been using it for a day with no issues :)

Cargo.toml back to CRLF

fredizzimo · 2023-03-05T13:06:31Z

Can you make sure that everything still works properly when the size has been stored as pixels, and then going to an older version of Neovide? There's this report #1788, and I'm fairly sure it's caused by this MR.

Using a different name for the setting would probably be sufficient.

…tting

chomosuke · 2023-03-07T07:37:35Z

@fredizzimo done and tested :)

chomosuke · 2023-03-08T12:13:29Z

Is there anything that prevents this PR from being merged? It's been sitting here for a while now.

A-Lamia · 2023-03-08T13:57:38Z

as some one who has been using this patch for over 2 months on windows at least it is very solid.

chomosuke · 2023-03-10T01:04:35Z

as some one who has been using this patch for over 2 months on windows at least it is very solid.

Same, I've been using this on Linux and haven't encountered a single problem so far.

fredizzimo

Except for the tiny comment about a use statement, everything looks good.

I have also been annoyed with the resizing myself, so I would personally really appreciate if this got merged.

fredizzimo · 2023-04-08T00:58:33Z

src/settings/window_size.rs

@@ -1,5 +1,6 @@
 use std::path::PathBuf;

+use glutin::dpi::PhysicalSize;


This sould be winit::dpi::PyshicalSize, since already changed all the other references. winit::dpi::PhysicalPosition is also used just below this.

danielb2 · 2023-04-13T01:08:17Z

@MultisampledNight
@last-partizan hi is there any reason this isn't being reviewed and merged?

last-partizan · 2023-04-13T06:13:20Z

I would really prefer to have original --geometry unchanged, and store only window size in pixels. Is that possible?

chomosuke · 2023-04-13T13:13:45Z

@last-partizan The --geometry option still exist and is still usable. I'm not sure why would it be better than the new --size option though?

last-partizan · 2023-04-13T13:16:17Z

Oh, i missed that. I was thinking it was replaced. Looks good to me, then.

@Kethku what do you think, should we merge this?

Kethku · 2023-04-13T22:10:48Z

@MultisampledNight @last-partizan hi is there any reason this isn't being reviewed and merged?

https://neovide.dev/maintainer-cookbook.html#how-to-keep-your-sanity multisampled wrote this out and I think captures how us maintainers have needed to work on the project. We all have lives and jobs outside of it, and if we tried to track and respond to everything, we would burn out. At least thats how I personally see it.

I personally haven't reviewed this yet because it wasn't clear to me if this is the better solution over storing a grid size. I think the growing window problem likely comes from a disconnect between the stored size which is maybe an internal window size without the frame, and the set size which maybe includes the frame. Or visa versa. Storing a pixel size might be worse if the window was stored on display with a given scaling factor and then restored in a window with a different factor? Idk.

None of above should be a blocker, it was just some thoughts I had that weren't fully investigated. That lack of clarity on the "correct" solution stopped me personally from checking it off or reviewing.

I don't have a strong feeling either way though and this likely improves things so maybe its worth merging. @chomosuke do you have some thoughts on what I've written above? If you're confident I'm over worried Im happy to merge. I appreciate the persistence <3

chomosuke · 2023-04-14T04:16:24Z

No worries @Kethku, I totally understand the maintaining a project like this is hard work and I thank you for everything you've already done for neovide!

For the root cause of growing window I believe that it is caused by font size changing right after nvim starts, there's some further info on a comment I made a while ago and some evidence to support my beliefs. Overall storing the size in pixel has fixed the issue for me :).

I'm not sure about changing scaling factor though, I haven't tested that but regardless I think this PR is still an improvement that fixes the changing font issue and I do think it is worth merging.

Btw, if you think having more maintainer for this project will help, I am happy to be another maintainer. I have a few improvements to neovide in my mind and once I've made them I'd be more well versed in the code base and reviewing PR etc.

Kethku · 2023-04-14T04:51:27Z

Sold! You're probably right about the actual core issue as I had forgotten about the timing issue between reading the config and first window render. Thanks for the attention to detail. Maybe come hang out in the discord? That's where most of the maintainers are atm

* Cargo.toml unix line ending * added --size * figure out initial size in draw_frame() * refactor: Rename window_geometry -> window_size & move parse_window_geometry * remember window size by pixel * updated doc * format * remove executable permission on random files * changed name of size setting to avoid conflict with old version of setting * Changed one glutin::dpi::PhysicalSize to winit

chomosuke added 9 commits December 29, 2022 15:56

Cargo.toml unix line ending

23ff827

added --size

0e726d6

figure out initial size in draw_frame()

89ee5b7

refactor: Rename window_geometry -> window_size

76f2857

& move parse_window_geometry

remember window size by pixel

1e3ff3c

updated doc

4f64a81

Merge branch 'main' into remember-windows-size

a87599c

format

39f691c

remove executable permission on random files

257d291

Merge branch 'main' into remember-windows-size

600b117

Cargo.toml back to CRLF

chomosuke force-pushed the remember-windows-size branch from 3884cf7 to 600b117 Compare February 23, 2023 21:23

Merge branch 'main' into remember-windows-size

d49c882

A-Lamia mentioned this pull request Mar 5, 2023

Windows crashes on start, "Could not create surface" #1788

Closed

changed name of size setting to avoid conflict with old version of se…

67850c7

…tting

chomosuke force-pushed the remember-windows-size branch from 4f9bc39 to 67850c7 Compare March 7, 2023 07:35

Merge remote-tracking branch 'upstream/main' into remember-windows-size

ead822b

fredizzimo approved these changes Apr 8, 2023

View reviewed changes

Changed one glutin::dpi::PhysicalSize to winit

39103ca

chomosuke mentioned this pull request Apr 12, 2023

Window size is increasing with every start of neovide #1636

Closed

Kethku merged commit daf5307 into neovide:main Apr 14, 2023
3 checks passed

chomosuke deleted the remember-windows-size branch April 14, 2023 04:55

fredizzimo mentioned this pull request Aug 1, 2023

Neovide panicked with the message 'Could not create surface'. (File: src/renderer/rendered_window.rs; Line: 82, Column: 6) on macOS #1835

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remember windows size via pixel #1706

Remember windows size via pixel #1706

chomosuke commented Dec 30, 2022

A-Lamia commented Jan 4, 2023 •

edited

chomosuke commented Jan 4, 2023

A-Lamia commented Jan 4, 2023

fredizzimo commented Mar 5, 2023

chomosuke commented Mar 7, 2023

chomosuke commented Mar 8, 2023

A-Lamia commented Mar 8, 2023

chomosuke commented Mar 10, 2023

fredizzimo left a comment

fredizzimo Apr 8, 2023

chomosuke Apr 10, 2023

danielb2 commented Apr 13, 2023

last-partizan commented Apr 13, 2023

chomosuke commented Apr 13, 2023 •

edited

last-partizan commented Apr 13, 2023 •

edited

Kethku commented Apr 13, 2023

chomosuke commented Apr 14, 2023

Kethku commented Apr 14, 2023

		@@ -1,5 +1,6 @@
		use std::path::PathBuf;

		use glutin::dpi::PhysicalSize;

Remember windows size via pixel #1706

Remember windows size via pixel #1706

Conversation

chomosuke commented Dec 30, 2022

What kind of change does this PR introduce?

Did this PR introduce a breaking change?

A-Lamia commented Jan 4, 2023 • edited

chomosuke commented Jan 4, 2023

A-Lamia commented Jan 4, 2023

fredizzimo commented Mar 5, 2023

chomosuke commented Mar 7, 2023

chomosuke commented Mar 8, 2023

A-Lamia commented Mar 8, 2023

chomosuke commented Mar 10, 2023

fredizzimo left a comment

Choose a reason for hiding this comment

fredizzimo Apr 8, 2023

Choose a reason for hiding this comment

chomosuke Apr 10, 2023

Choose a reason for hiding this comment

danielb2 commented Apr 13, 2023

last-partizan commented Apr 13, 2023

chomosuke commented Apr 13, 2023 • edited

last-partizan commented Apr 13, 2023 • edited

Kethku commented Apr 13, 2023

chomosuke commented Apr 14, 2023

Kethku commented Apr 14, 2023

A-Lamia commented Jan 4, 2023 •

edited

chomosuke commented Apr 13, 2023 •

edited

last-partizan commented Apr 13, 2023 •

edited