Skip to content

Move shared code to new crate winit-common#4241

Merged
kchibisov merged 1 commit intomasterfrom
madsmtm/winit-apple-common
May 25, 2025
Merged

Move shared code to new crate winit-common#4241
kchibisov merged 1 commit intomasterfrom
madsmtm/winit-apple-common

Conversation

@madsmtm
Copy link
Copy Markdown
Member

@madsmtm madsmtm commented May 20, 2025

This crate contains helper code that is useful for both the AppKit and the UIKit backends, as well as the shared XKB helper code.

Part of #3433.

  • Tested on all platforms changed

@madsmtm madsmtm added DS - appkit Affects the AppKit/macOS backend DS - uikit Affects the UIKit backend (iOS, tvOS, watchOS, visionOS) S - maintenance Repaying technical debt labels May 20, 2025
@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch from 5579a2f to 6a42da6 Compare May 20, 2025 14:55
@madsmtm madsmtm added this to the Version 0.31.0 milestone May 20, 2025
@madsmtm madsmtm force-pushed the madsmtm/workspace-deps branch from cefed24 to 96b66dc Compare May 20, 2025 19:39
@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch 2 times, most recently from c614ee3 to a83ee97 Compare May 20, 2025 19:52
@madsmtm madsmtm mentioned this pull request May 15, 2025
5 tasks
Base automatically changed from madsmtm/workspace-deps to master May 21, 2025 06:45
@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch from a83ee97 to 24a7d42 Compare May 21, 2025 10:48
@madsmtm madsmtm marked this pull request as ready for review May 21, 2025 11:03
@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch from 24a7d42 to b9bfa01 Compare May 21, 2025 11:20
@madsmtm madsmtm requested a review from kchibisov May 21, 2025 11:41
@kchibisov
Copy link
Copy Markdown
Member

One thing I'm not entirely sure is that whether we want winit-common and then dirs like apple/linux/etc inside of it or a crate per domain. Maybe winit-common with an apple module inside of it is better.

@madsmtm
Copy link
Copy Markdown
Member Author

madsmtm commented May 21, 2025

One thing I'm not entirely sure is that whether we want winit-common and then dirs like apple/linux/etc inside of it or a crate per domain. Maybe winit-common with an apple module inside of it is better.

I'm not sure either.

Building on the discussion in #4246, we might not even want to call it "linux" and "apple" - maybe rather winit_common::xkb + winit_common::core_foundation + winit_common::foundation?

@kchibisov
Copy link
Copy Markdown
Member

Yeah, that's true, so I'd probably be infavour of the common module to just add stuff that could be needed by multiple backends to e.g. write handling that does the same thing on every platform. So probably a big common crate for stuff we need is a better options.

@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch from b9bfa01 to 74d68ce Compare May 21, 2025 12:55
@madsmtm madsmtm requested a review from notgull as a code owner May 21, 2025 12:55
@madsmtm madsmtm force-pushed the madsmtm/winit-apple-common branch 3 times, most recently from ee80ce8 to 035b43d Compare May 21, 2025 13:02
@madsmtm
Copy link
Copy Markdown
Member Author

madsmtm commented May 21, 2025

I moved it to winit-common, along with the XKB stuff.

Check it out, and let me know what you think?

@madsmtm madsmtm changed the title Move shared Apple code to new crate winit-apple-common Move shared code to new crate winit-common May 21, 2025
@madsmtm madsmtm added DS - x11 Affects the X11 backend, or generally free Unix platforms DS - wayland Affects the Wayland backend, or generally free Unix platforms labels May 21, 2025
Copy link
Copy Markdown
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I'd probably still move apple specific stuff into separate folder, like event_handler for now. We can also use the build.rs stuff we have to gate features that can not really build on platforms.

@madsmtm
Copy link
Copy Markdown
Member Author

madsmtm commented May 21, 2025

I'd probably still move apple specific stuff into separate folder, like event_handler for now.

I put the event_handler at the top-level, because I plan to use it in the Web backend too.

We can also use the build.rs stuff we have to gate features that can not really build on platforms.

But the CoreFoundation stuff can build on more platforms - e.g. if you have certain GNUStep libraries installed, you can build winit-common/core_foundation on Linux.

Similarly, e.g. cargo build -pwinit-common --features wayland,xkb --target aarch64-apple-darwin works perfectly fine.

@kchibisov
Copy link
Copy Markdown
Member

maybe group in objc it then, so it's a bit clear what it is about?

@madsmtm
Copy link
Copy Markdown
Member Author

madsmtm commented May 21, 2025

I guess I'm also fine with:

  • xkb
  • apple::EventLoopProxy - where "Apple" here would indicate the API, but not necessarily restrict it to Apple platforms only?
  • interior_mutable::EventHandler?

(objc is wrong, since none of these APIs actually use Objective-C calls, they just use crates from the objc2 project).

@nicoburns
Copy link
Copy Markdown
Contributor

nicoburns commented May 21, 2025

But the CoreFoundation stuff can build on more platforms - e.g. if you have certain GNUStep libraries installed, you can build winit-common/core_foundation on Linux.

Can the AppKit backend be used on Linux? If not, then what's the point of the core_foundation code on Linux? A hypothetical separate GNUStep backend? (we don't want to compile any more code than is strictly necessary)

@madsmtm
Copy link
Copy Markdown
Member Author

madsmtm commented May 21, 2025

Can the AppKit backend be used on Linux?

Probably not functionally yet, but I think it'd be interesting to move towards that, yes.

(GNUStep contains CoreFoundation/Foundation/AppKit reimplementations).

(we don't want to compile any more code than is strictly necessary)

I've cfg-gated it already ;).

@kchibisov kchibisov force-pushed the madsmtm/winit-apple-common branch 2 times, most recently from 6e0a6ce to a22e93c Compare May 25, 2025 11:07
@kchibisov kchibisov force-pushed the madsmtm/winit-apple-common branch from a22e93c to 35b70c4 Compare May 25, 2025 11:12
@kchibisov kchibisov merged commit 0adc089 into master May 25, 2025
113 of 114 checks passed
@kchibisov kchibisov deleted the madsmtm/winit-apple-common branch May 25, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DS - appkit Affects the AppKit/macOS backend DS - uikit Affects the UIKit backend (iOS, tvOS, watchOS, visionOS) DS - wayland Affects the Wayland backend, or generally free Unix platforms DS - x11 Affects the X11 backend, or generally free Unix platforms S - maintenance Repaying technical debt

Development

Successfully merging this pull request may close these issues.

3 participants