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

On wasm, provide intradoc-link for spawn() function in EventLoop docs #3178

Merged
merged 1 commit into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,22 @@ impl<T> EventLoop<T> {
/// (that Rust doesn't see) that will also mean that the rest of the function is never executed
/// and any values not passed to this function will *not* be dropped.
///
/// Web applications are recommended to use `spawn()` instead of `run()` to avoid the need
/// Web applications are recommended to use
#[cfg_attr(
wasm_platform,
doc = "[`EventLoopExtWebSys::spawn()`][crate::platform::web::EventLoopExtWebSys::spawn()]"
daxpedda marked this conversation as resolved.
Show resolved Hide resolved
)]
#[cfg_attr(not(wasm_platform), doc = "`EventLoopExtWebSys::spawn()`")]
/// [^1] instead of [`run()`] to avoid the need
/// for the Javascript exception trick, and to make it clearer that the event loop runs
/// asynchronously (via the browser's own, internal, event loop) and doesn't block the
/// current thread of execution like it does on other platforms.
///
/// This function won't be available with `target_feature = "exception-handling"`.
///
/// [`set_control_flow()`]: EventLoopWindowTarget::set_control_flow
/// [`set_control_flow()`]: EventLoopWindowTarget::set_control_flow()
/// [`run()`]: Self::run()
Copy link
Member Author

@MarijnS95 MarijnS95 Oct 24, 2023

Choose a reason for hiding this comment

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

Not super happy with this, because it'll fail to link on not(all(wasm_platform, target_feature = "exception-handling")) (which is strange, because features must be additive and should not remove functionality).

Copy link
Member

Choose a reason for hiding this comment

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

target_feature = "exception-handling" isn't stable, which is probably why it didn't work for you.

Copy link
Member Author

Choose a reason for hiding this comment

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

@daxpedda reading this again, I missed target_ and assumed it was just feature = (from the [features] list in Cargo.toml).

I think the feature must have been _en_abled (running nightly rustdoc for --cfg docsrs => doc_cfg), as it's the only way to cause not(all(true, true)) = not(true) = false and turn off fn run()?

Copy link
Member

Choose a reason for hiding this comment

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

That is weird, target_feature = "exception-handling" should not automatically have been enabled by running nightly rustdoc.

/// [^1]: `EventLoopExtWebSys::spawn()` is only available on WASM.
#[inline]
#[cfg(not(all(wasm_platform, target_feature = "exception-handling")))]
pub fn run<F>(self, event_handler: F) -> Result<(), EventLoopError>
Expand Down
46 changes: 32 additions & 14 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,36 @@
//! window or a key getting pressed while the window is focused. Devices can generate
//! [`DeviceEvent`]s, which contain unfiltered event data that isn't specific to a certain window.
//! Some user activity, like mouse movement, can generate both a [`WindowEvent`] *and* a
//! [`DeviceEvent`]. You can also create and handle your own custom [`UserEvent`]s, if desired.
//! [`DeviceEvent`]. You can also create and handle your own custom [`Event::UserEvent`]s, if desired.
//!
//! You can retrieve events by calling [`EventLoop::run`][event_loop_run]. This function will
//! You can retrieve events by calling [`EventLoop::run()`]. This function will
//! dispatch events for every [`Window`] that was created with that particular [`EventLoop`], and
//! will run until [`exit()`] is used, at which point [`Event`]`::`[`LoopExiting`].
//! will run until [`exit()`] is used, at which point [`Event::LoopExiting`].
//!
//! Winit no longer uses a `EventLoop::poll_events() -> impl Iterator<Event>`-based event loop
//! model, since that can't be implemented properly on some platforms (e.g web, iOS) and works poorly on
//! most other platforms. However, this model can be re-implemented to an extent with
//! [`EventLoopExtPumpEvents::pump_events`]. See that method's documentation for more reasons about why
#![cfg_attr(
any(
windows_platform,
macos_platform,
android_platform,
x11_platform,
wayland_platform
),
doc = "[`EventLoopExtPumpEvents::pump_events()`][platform::pump_events::EventLoopExtPumpEvents::pump_events()]"
daxpedda marked this conversation as resolved.
Show resolved Hide resolved
)]
#![cfg_attr(
not(any(
windows_platform,
macos_platform,
android_platform,
x11_platform,
wayland_platform
)),
doc = "`EventLoopExtPumpEvents::pump_events()`"
)]
//! [^1]. See that method's documentation for more reasons about why
//! it's discouraged, beyond compatibility reasons.
//!
//!
Expand Down Expand Up @@ -92,16 +112,16 @@
//! });
//! ```
//!
//! [`Event`]`::`[`WindowEvent`] has a [`WindowId`] member. In multi-window environments, it should be
//! compared to the value returned by [`Window::id()`][window_id_fn] to determine which [`Window`]
//! [`WindowEvent`] has a [`WindowId`] member. In multi-window environments, it should be
//! compared to the value returned by [`Window::id()`] to determine which [`Window`]
//! dispatched the event.
//!
//! # Drawing on the window
//!
//! Winit doesn't directly provide any methods for drawing on a [`Window`]. However it allows you to
//! retrieve the raw handle of the window and display (see the [`platform`] module and/or the
//! [`raw_window_handle`] and [`raw_display_handle`] methods), which in turn allows
//! you to create an OpenGL/Vulkan/DirectX/Metal/etc. context that can be used to render graphics.
//! you to create an OpenGL/Vulkan/DirectX/Metal/etc. context that can be used to render graphics.
//!
//! Note that many platforms will display garbage data in the window's client area if the
//! application doesn't render anything to the window by the time the desktop compositor is ready to
Expand All @@ -110,25 +130,23 @@
//! window visible only once you're ready to render into it.
//!
//! [`EventLoop`]: event_loop::EventLoop
//! [`EventLoopExtPumpEvents::pump_events`]: ./platform/pump_events/trait.EventLoopExtPumpEvents.html#tymethod.pump_events
//! [`EventLoop::new()`]: event_loop::EventLoop::new
//! [event_loop_run]: event_loop::EventLoop::run
//! [`EventLoop::run()`]: event_loop::EventLoop::run
//! [`exit()`]: event_loop::EventLoopWindowTarget::exit
//! [`Window`]: window::Window
//! [`WindowId`]: window::WindowId
//! [`WindowBuilder`]: window::WindowBuilder
//! [window_new]: window::Window::new
//! [window_builder_new]: window::WindowBuilder::new
//! [window_builder_build]: window::WindowBuilder::build
//! [window_id_fn]: window::Window::id
//! [`Event`]: event::Event
//! [`Window::id()`]: window::Window::id
//! [`WindowEvent`]: event::WindowEvent
//! [`DeviceEvent`]: event::DeviceEvent
//! [`UserEvent`]: event::Event::UserEvent
//! [`LoopExiting`]: event::Event::LoopExiting
//! [`platform`]: platform
//! [`Event::UserEvent`]: event::Event::UserEvent
//! [`Event::LoopExiting`]: event::Event::LoopExiting
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want this PR to turn into a "let's fix all winit intradoc links", but a few more changes are still needed to make this consistent.

No clue what the point is with manual relative .html links that may not resolve when certain features are not turned on... Pointing the user to a 404 page :(

Copy link
Member

Choose a reason for hiding this comment

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

This is probably a problem for another time, but these methods are deprecated anyway.

//! [`raw_window_handle`]: ./window/struct.Window.html#method.raw_window_handle
//! [`raw_display_handle`]: ./window/struct.Window.html#method.raw_display_handle
//! [^1]: `EventLoopExtPumpEvents::pump_events()` is only available on Windows, macOS, Android, X11 and Wayland.

#![deny(rust_2018_idioms)]
#![deny(rustdoc::broken_intra_doc_links)]
Expand Down
15 changes: 10 additions & 5 deletions src/platform/run_on_demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ pub trait EventLoopExtRunOnDemand {
/// See the [`set_control_flow()`] docs on how to change the event loop's behavior.
///
/// # Caveats
/// - This extension isn't available on all platforms, since it's not always possible to
/// return to the caller (specifically this is impossible on iOS and Web - though with
/// the Web backend it is possible to use `spawn()` more than once instead).
/// - This extension isn't available on all platforms, since it's not always possible to return
/// to the caller (specifically this is impossible on iOS and Web - though with the Web
/// backend it is possible to use `EventLoopExtWebSys::spawn()`[^1] more than once instead).
/// - No [`Window`] state can be carried between separate runs of the event loop.
///
/// You are strongly encouraged to use [`EventLoop::run()`] for portability, unless you specifically need
Expand All @@ -57,8 +57,13 @@ pub trait EventLoopExtRunOnDemand {
/// on an event loop that is internal to the browser itself.
/// - **iOS:** It's not possible to stop and start an `NSApplication` repeatedly on iOS.
///
/// [`exit()`]: EventLoopWindowTarget::exit
/// [`set_control_flow()`]: EventLoopWindowTarget::set_control_flow
#[cfg_attr(
not(wasm_platform),
doc = "[^1]: `spawn()` is only available on `wasm` platforms."
)]
///
/// [`exit()`]: EventLoopWindowTarget::exit()
/// [`set_control_flow()`]: EventLoopWindowTarget::set_control_flow()
fn run_on_demand<F>(&mut self, event_handler: F) -> Result<(), EventLoopError>
where
F: FnMut(Event<Self::UserEvent>, &EventLoopWindowTarget<Self::UserEvent>);
Expand Down
19 changes: 17 additions & 2 deletions src/platform/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,28 @@ pub trait EventLoopExtWebSys {

/// Initializes the winit event loop.
///
/// Unlike `run`, this returns immediately, and doesn't throw an exception in order to
/// satisfy its `!` return type.
/// Unlike
#[cfg_attr(
all(wasm_platform, target_feature = "exception-handling"),
doc = "`run()`"
)]
#[cfg_attr(
not(all(wasm_platform, target_feature = "exception-handling")),
doc = "[`run()`]"
)]
/// [^1], this returns immediately, and doesn't throw an exception in order to
/// satisfy its [`!`] return type.
///
/// Once the event loop has been destroyed, it's possible to reinitialize another event loop
/// by calling this function again. This can be useful if you want to recreate the event loop
/// while the WebAssembly module is still loaded. For example, this can be used to recreate the
/// event loop when switching between tabs on a single page application.
///
#[cfg_attr(
not(all(wasm_platform, target_feature = "exception-handling")),
doc = "[`run()`]: EventLoop::run()"
)]
/// [^1]: `run()` is _not_ available on WASM when the target supports `exception-handling`.
fn spawn<F>(self, event_handler: F)
where
F: 'static + FnMut(Event<Self::UserEvent>, &EventLoopWindowTarget<Self::UserEvent>);
Expand Down