Skip to content

Conversation

@RossComputerGuy
Copy link

No description provided.

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Two things:

  • raw_window_handle is intended to be a common types crate to be used by higher-level crates, rather than something that implements functionality on its own. So I would rather have it represent a Handle as a NonNull<T> since it's a raw pointer.
  • From a quick glance at the docs, it seems that GraphicsOutput has a framebuffer of its own. So I think it would be better if GraphicsOutput was a property of the WindowHandle rather than the DisplayHandle.

src/lib.rs Outdated
///
/// ## Availability Hints
/// This variant is used on UEFI.
#[cfg(target_os = "uefi")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[cfg(target_os = "uefi")]

We avoid target conditionals here.

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/lib.rs Outdated
from_impl!(RawDisplayHandle, Android, AndroidDisplayHandle);
from_impl!(RawDisplayHandle, Haiku, HaikuDisplayHandle);

#[cfg(target_os = "uefi")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[cfg(target_os = "uefi")]

Ditto

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/uefi.rs Outdated
Comment on lines 28 to 48
impl DisplayHandle<'static> {
/// Create a UEFI-based display handle.
///
/// As no data is borrowed by this handle, it is completely safe to create. This function
/// may be useful to windowing framework implementations that want to avoid unsafe code.
///
/// # Example
///
/// ```
/// # use raw_window_handle::{DisplayHandle, HasDisplayHandle};
/// # fn do_something(rwh: impl HasDisplayHandle) { let _ = rwh; }
/// let handle = DisplayHandle::uefi();
/// do_something(handle);
/// ```
pub fn uefi() -> Result<Self, HandleError> {
let proto = boot::get_handle_for_protocol::<GraphicsOutput>()
.or_else(|_| Err(HandleError::Unavailable))?;
// SAFETY: No data is borrowed.
Ok(unsafe { Self::borrow_raw(UefiDisplayHandle::new(proto).into()) })
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
impl DisplayHandle<'static> {
/// Create a UEFI-based display handle.
///
/// As no data is borrowed by this handle, it is completely safe to create. This function
/// may be useful to windowing framework implementations that want to avoid unsafe code.
///
/// # Example
///
/// ```
/// # use raw_window_handle::{DisplayHandle, HasDisplayHandle};
/// # fn do_something(rwh: impl HasDisplayHandle) { let _ = rwh; }
/// let handle = DisplayHandle::uefi();
/// do_something(handle);
/// ```
pub fn uefi() -> Result<Self, HandleError> {
let proto = boot::get_handle_for_protocol::<GraphicsOutput>()
.or_else(|_| Err(HandleError::Unavailable))?;
// SAFETY: No data is borrowed.
Ok(unsafe { Self::borrow_raw(UefiDisplayHandle::new(proto).into()) })
}
}

This level of functionality is too high for this crate.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@RossComputerGuy
Copy link
Author

  • From a quick glance at the docs, it seems that GraphicsOutput has a framebuffer of its own. So I think it would be better if GraphicsOutput was a property of the WindowHandle rather than the DisplayHandle.

The framebuffer is only available when the Graphics Output Protocol implements a framebuffer mode. Otherwise, you have to use blit mode.

@notgull
Copy link
Member

notgull commented Oct 5, 2025

Still, you draw to the GraphicsOutput, which in line with our earlier implementation for DRM/KMS. I think if you draw to it, it should be classified as a window.

@RossComputerGuy
Copy link
Author

I'm not sure what you mean. When drawing in UEFI, you use a temporary buffer which you render to. Then whenever you present, you copy from that buffer to the memory mapped one if it is available. Otherwise, you use the blit function in the protocol. If the memory mapped buffer is not available, that means the GPU just doesn't give the ability for direct screen access.

I was wanting to use this with something like iced and have it like a compositor. Have multiple windows which get drawn and be able to manipulate them. At least in this crate and softbuffer, it shouldn't be anything special to make that possible.

@RossComputerGuy RossComputerGuy force-pushed the feat/uefi branch 2 times, most recently from 9a2e5b5 to ebc69d4 Compare October 5, 2025 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants