Skip to content

Conversation

DockFrankenstein
Copy link

@DockFrankenstein DockFrankenstein commented Jul 20, 2025

PR Details

This PR fixes a crash that would occur when trying to resize a window on OpenGL, other issues with resizing the depth buffer, a fix to GraphicsAdapter.Outputs not being populated with the right values and a refactor to how SDL window resize events are handled

Related PRs

Related Issue

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@Eideren
Copy link
Collaborator

Eideren commented Aug 28, 2025

@DockFrankenstein the inconsistent behavior issue you linked has been commented on yesterday, any thoughts on what they said ?

@DockFrankenstein
Copy link
Author

@Eideren Sorry, I'm not sure which issue you are talking about

@Eideren
Copy link
Collaborator

Eideren commented Aug 28, 2025

This one libsdl-org/SDL#13149

@DockFrankenstein
Copy link
Author

Okay I read it. Seems like this is a "sort of" intended behavior. I'm gonna have to look into this more and hopefully create a better, more proper fix

@DockFrankenstein DockFrankenstein marked this pull request as draft August 29, 2025 08:31
Copy link
Contributor

@Ethereal77 Ethereal77 left a comment

Choose a reason for hiding this comment

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

I'm not very knowledgeable in OpenGL, and I cannot test these changes right now, but if it works and has been tested, it looks good enough for me. Just left some comments.

Maybe the weird order in which SDL calls ResizeBeginActions is related with the other issue in which the SDL backend does weird things for full-screen mode? I don't know for suer. Ignore this if not.

@DockFrankenstein DockFrankenstein marked this pull request as ready for review September 1, 2025 17:13
@DockFrankenstein
Copy link
Author

After a comment from @Eideren, I decided to look into how the engine handles resizing to figure out why I had to do the original hack in the first place. I already explained the issue in the discord server, but the gist is: previously, we incorrectly assumed what SizeChanged and Resized events in SDL did based on how they behaved in windows.

Here is the correct description:

  • SizeChanged - when the window changes its size in any way (user resizing, via code, or some other way by the desktop environment)
  • Resized - when the window is manually resized by the user (note: alt+enter doesn't trigger it)

References: [ forum post ] [ sdl documentation ]

Copy link
Contributor

@Ethereal77 Ethereal77 left a comment

Choose a reason for hiding this comment

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

Looks good. Just that you forgot to change the comment.

@Ethereal77
Copy link
Contributor

Do you think there's something that could be done to the SDL backend to better convey the difference? Or to better document it?
I think maybe the SDL backend is in need of some love by someone that knows about it.

In the future, when Silk supports SDL3, it should be rewritten or improved.

@DockFrankenstein
Copy link
Author

@Ethereal77 I'm not sure. I think just changing the names of the events is enough. We could perhaps write a better summary comment for those

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants