Skip to content

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

Merged
madsmtm merged 3 commits intomasterfrom
madsmtm/no-fake-scalefactorchanged
Dec 2, 2024
Merged

AppKit/UIKit: Do not emit default state events upon window creation#3912
madsmtm merged 3 commits intomasterfrom
madsmtm/no-fake-scalefactorchanged

Conversation

@madsmtm
Copy link
Copy Markdown
Member

@madsmtm madsmtm commented Sep 11, 2024 •
edited
Loading

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 - appkit Affects the AppKit/macOS backend DS - uikit Affects the UIKit backend (iOS, tvOS, watchOS, visionOS) S - platform parity Unintended platform differences labels Sep 11, 2024
madsmtm
madsmtm commented Sep 11, 2024
View reviewed changes
src/platform_impl/apple/uikit/window.rs
Comment on lines -513 to -521
// Like the Windows and macOS backends, we send a `ScaleFactorChanged` and `SurfaceResized`
// event on window creation if the DPI factor != 1.0
Copy link
Copy Markdown
Member Author

@madsmtm madsmtm Sep 11, 2024

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
Copy Markdown
Member

kchibisov commented Sep 11, 2024

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
Copy Markdown
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
Copy Markdown
Member

kchibisov commented Sep 11, 2024

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
Copy Markdown
Member Author

madsmtm commented Sep 11, 2024

Ah, nice, thanks!

@madsmtm madsmtm mentioned this pull request Sep 11, 2024
Merged
2 tasks
@madsmtm madsmtm merged commit cfa8f02 into master Dec 2, 2024
@madsmtm madsmtm deleted the madsmtm/no-fake-scalefactorchanged branch December 2, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kchibisov kchibisov kchibisov approved these changes

Assignees

No one assigned

Labels

B - bug Dang, that shouldn't have happened DS - appkit Affects the AppKit/macOS backend DS - uikit Affects the UIKit backend (iOS, tvOS, watchOS, visionOS) S - platform parity Unintended platform differences

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@madsmtm @kchibisov