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

Increase GFX output buffer size #2969

Merged
merged 1 commit into from
Mar 3, 2024

Conversation

matt335672
Copy link
Member

Fixes #2968

Size the GFX output buffer pessimistically based on the largest monitor in the configuration.

A couple of comments:-

  1. The sizing is really pessimistic. It assumes that no compression happens to the incoming data at all.
  2. This may not be needed - see this comment from @jsorg71

Because of 2) above, this is still in draft.

Size the GFX output buffer pessimistically based on the largest monitor
in the configuration.
@metalefty
Copy link
Member

The patch 1) works great for me.

@metalefty
Copy link
Member

Can this be merged? This is needed for v0.10 to continue the user acceptance test.
Let's merge another PR to devel if this is not needed (case 2).

@matt335672
Copy link
Member Author

If it's needed for user acceptance testing, I think we should merge it as you suggest.

@matt335672 matt335672 marked this pull request as ready for review March 2, 2024 17:52
@Nexarian
Copy link
Contributor

Nexarian commented Mar 2, 2024

I think @jsorg71 was investigating an anomaly that this shouldn't be necessary; but if it fixes a user facing issue then we should still merge it. No objections here.

@jsorg71
Copy link
Contributor

jsorg71 commented Mar 2, 2024

@matt335672 I think we still need this fix. I just want to figure out why it crashed instead of just returning a few less tiles.
I did try to reproduce this crash and I can not. I don't have a 4k screen but I set the max buffer to 1000 and I got one tile back which is expected. Anyway lets not hold this up.

@metalefty metalefty merged commit b6d4e9d into neutrinolabs:devel Mar 3, 2024
13 checks passed
@metalefty
Copy link
Member

Thanks, merging and will cherry-pick to v0.10.

@matt335672 matt335672 deleted the increase_gfx_buffer branch March 3, 2024 12:47
@matt335672
Copy link
Member Author

Agreed that this needs more investigation. I've re-opened the original fault report which was closed automatically.

@jsorg71 - I don't have a 4k monitor either, but I managed to frig something with the console on a windows VM which was bigger than my virtual monitor. Also, I think that you can probably just edit a mstsc ini file which isn't full sceen, and just put the larger dimensions in. I don't have access to an environment to check this today though.

@jsorg71
Copy link
Contributor

jsorg71 commented Mar 4, 2024

Thanks Matt, I can repro and I found the issue. I'll test a couple of days, then do a PR.

@matt335672
Copy link
Member Author

That's great news @jsorg71.

Can you undo 90e4aca as part of the PR? I was never really happy with it as anything other than a short-term fix.

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.

GFX connection reliably crashes with SIGABRT
4 participants