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

AppKit/UIKit: Do not emit default state events upon window creation #3912

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 11, 2024

In Winit, we generally do not emit events for the initial value of things on the window. Existing examples include ThemeChanged, ModifiersChanged, Moved, CursorMoved and so on. So let's not do that for ScaleFactorChanged, SurfaceResized and Focused either!

See also #2696 and #2585 for some of the history on Focused. I don't know what I was thinking in #2585 (comment), but it's clearly wrong, we don't need to emit Focused(false) since it's the default for all platforms.

Part of #2640.

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior

@madsmtm madsmtm added B - bug Dang, that shouldn't have happened DS - macos DS - ios S - platform parity Unintended platform differences labels Sep 11, 2024
Comment on lines -513 to -514
// Like the Windows and macOS backends, we send a `ScaleFactorChanged` and `SurfaceResized`
// event on window creation if the DPI factor != 1.0
Copy link
Member Author

Choose a reason for hiding this comment

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

This comment does not seem to be true any more, the code for the Windows backend to do this seems to have been removed.

@kchibisov
Copy link
Member

As long as the values are the same as the user queried them at the point of create_window it should be fine to not emit those, since the events imply that something default got changed, when it clearly not changed.

@madsmtm madsmtm force-pushed the madsmtm/no-fake-scalefactorchanged branch from 88ecc68 to cd8a062 Compare September 11, 2024 15:07
@madsmtm
Copy link
Member Author

madsmtm commented Sep 11, 2024

I have updated since you looked to also remove the redundant Focused(false) event on macOS that I added myself in #2585 (comment).

@madsmtm madsmtm changed the title AppKit/UIKit: Do not emit ScaleFactorChanged upon window creation AppKit/UIKit: Do not emit default state events upon window creation Sep 11, 2024
@kchibisov
Copy link
Member

The focus stuff was required before since it wasn't stated whether the window is focused by default or not, so some assumed yeS, others not, so it was a bit messy.

I know that right now we explicitly declare that the window is not focused by default.

@madsmtm
Copy link
Member Author

madsmtm commented Sep 11, 2024

Ah, nice, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - ios DS - macos S - platform parity Unintended platform differences
Development

Successfully merging this pull request may close these issues.

2 participants