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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Add minimum sizes to textures to prevent crash #2300

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Add minimum sizes to textures to prevent crash #2300

wants to merge 1 commit into from

Conversation

trolleyman
Copy link
Contributor

Objective

Solution

  • Ensures that textures are never requested with 0 height/width.

@trolleyman trolleyman mentioned this pull request Jun 3, 2021
Closed
@bjorn3
Copy link
Contributor

bjorn3 commented Jun 3, 2021

I think the proper solution would be to simply not create a swapchain and not render anything when the window height is 0. Setting a minimum size of 1 for textures could mask a user error.

@mockersf
Copy link
Member

mockersf commented Jun 3, 2021

Related to #545 and #734 that tried to stop creating a texture of size (0,0) when minimizing the window.

@NathanSWard NathanSWard added A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash A-Windowing Platform-agnostic interface layer to run your app in labels Jun 3, 2021
@NathanSWard
Copy link
Contributor

NathanSWard commented Jun 3, 2021 •
edited
Loading

To quote cart:

While this works, its similar to the approach used in #545. I think I'd prefer to let the window correctly report whatever its size is, then "fix" the source of the crash directly in bevy_wgpu (by disallowing zero-width/height textures). #545 (comment)

So similar to as @bjorn3 said, the solution here is not to .max(1)

@cart
Copy link
Member

cart commented Jun 3, 2021 •
edited
Loading

I think the approach in this pr is an improvement over #545 and #734 because it works around a wgpu-specific constraint inside bevy_wgpu.

I agree with @bjorn3 that ideally we just don't render anything, but thats actually non-trivial to do currently in the context of the RenderGraph. Theres no way for the window swap chain node to say "I can't create a swap chain, so don't run any of my dependent nodes". Arguably that should be possible, but its a new feature that needs to be built.

I think we should merge this pr as a "quick fix" interim solution to a very common problem, with a TODO outlining the ideal solution and a reference to an issue capturing the ideal solution.

@TheRawMeatball TheRawMeatball mentioned this pull request Jun 6, 2021
Closed
@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 9, 2021
@bors
Try #2300:
80f5b1b
@bors
Copy link
Contributor

bors bot commented Jun 9, 2021

try

Build succeeded:

@cart
Copy link
Member

cart commented Jun 9, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 9, 2021
@trolleyman
Add minimum sizes to textures to prevent crash (#2300)
a40ec1c 
# Objective
- Fixes #2299

## Solution
- Ensures that textures are never requested with 0 height/width.
@bors
Copy link
Contributor

bors bot commented Jun 9, 2021

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Add minimum sizes to textures to prevent crash [Merged by Bors] - Add minimum sizes to textures to prevent crash Jun 9, 2021
@bors bors bot closed this Jun 9, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
@trolleyman @ostwilkens
Add minimum sizes to textures to prevent crash (bevyengine#2300)
eb63a27 
# Objective
- Fixes bevyengine#2299

## Solution
- Ensures that textures are never requested with 0 height/width.
@alice-i-cecile alice-i-cecile mentioned this pull request Dec 4, 2021
Open
@trolleyman trolleyman deleted the fix-minimize-crash branch July 1, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen A-Windowing Platform-agnostic interface layer to run your app in P-Crash A sudden unexpected crash
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

window: (minimize, 0 size, oom) crashes
6 participants
@trolleyman @bjorn3 @mockersf @NathanSWard @cart @alice-i-cecile