unom/punktfunk
6
0
Fork 1
Code Issues 2 Pull Requests Actions 2 Packages Projects Releases 40 Wiki Activity

refactor(capture/windows): eleven D3D11 helpers had no caller contract

Browse Source 
The DXGI converters (`HdrP010Converter`, `BgraToYuvPlanes`, `VideoConverter`), the cursor blend
pass, and `create_ring_slots` declared their methods `unsafe fn` because their bodies are D3D11
FFI. That is a property of the body, not an obligation on the caller, and their own proofs said so:
"`?`-checked D3D11 methods on the live `device` borrow". Every parameter is either a borrowed
windows-rs COM wrapper (`&ID3D11Device`, `&ID3D11DeviceContext`, `&ID3D11Texture2D`, the views) or
a plain `u32`/`bool`/`DXGI_FORMAT`; each body builds its own descriptors from those and every
interface it creates owns its reference. There is nothing a caller can pass that makes them
unsound.

Not one of the eleven carried a `# Safety` section — the marker had no stated contract to inherit,
which is the tell. Each body was already one `unsafe {}` with its proof, so this is purely
subtractive: no block moved and no proof was written. The four call sites lost `unsafe {}` wrappers
whose own comments gave the game away — "`X::new` is `unsafe` (it compiles D3D11 shaders…)",
"`create_ring_slots` is an `unsafe fn` (it makes D3D11/DXGI COM calls)" — comments explaining that
the marker described the callee's body rather than anything the caller had to guarantee.

`compile_shader` KEEPS its `unsafe fn`: it takes `PCSTR`, a raw pointer the caller must guarantee
points at a NUL-terminated literal. That is the line — a contract a caller can actually break.
pf-zerocopy was surveyed the same way and deliberately left alone: its CUDA/Vulkan helpers look
identical by signature but state real obligations ("the shared context is current", "single-threaded
use of handles this bridge owns"), so signature screening finds candidates and only reading decides.

pf-capture already denies both `unsafe_op_in_unsafe_fn` and `undocumented_unsafe_blocks`, so the
crate stays fully enforced; the reduction is in obligations, not in warnings. It now has exactly one
`unsafe fn` left. Verified on the Intel box .47: pf-capture clippy `-D warnings` rc=0 and its 18
Windows tests pass, with host/pf-encode/pf-vdisplay clippy still green; Linux .21 fmt + both CI
clippy steps rc=0.
This commit is contained in:
enricobuehler
2026-07-29 00:20:35 +02:00
parent 50531c8e9e
commit 9e8473b46a
4 changed files with 38 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-capture/src/windows/dxgi.rs
+16 -11
View File
@@ -387,11 +387,20 @@ pub(crate) struct HdrP010Converter {
cbuf: ID3D11Buffer,
}
// The three converters' methods below are SAFE fns. They were `unsafe fn` because their bodies are
// D3D11 FFI, which is not the same thing as having a caller contract: every parameter is a borrowed
// windows-rs COM wrapper (`&ID3D11Device`, `&ID3D11DeviceContext`, `&ID3D11Texture2D`, the views) or
// a plain `u32`/`bool`, each body builds its own descriptors from those, and every created interface
// owns its reference. There is nothing a caller can pass that makes them unsound, and the proofs the
// markers carried said exactly that — "`?`-checked D3D11 methods on the live `device` borrow" — which
// is a description of the body, not an obligation. `unsafe` now marks the FFI inside them, where the
// blocks and their proofs already were. `compile_shader` KEEPS its marker: it takes `PCSTR`, a raw
// pointer the caller must guarantee is a NUL-terminated literal.
impl HdrP010Converter {
/// `w`/`h` are the SOURCE dimensions this converter's chroma pass will sample, baked into the
/// immutable constant buffer. Rebuild the converter if they change (the IDD capturer's
/// `recreate_ring` already drops it).
pub(crate) unsafe fn new(device: &ID3D11Device, w: u32, h: u32) -> Result<Self> {
pub(crate) fn new(device: &ID3D11Device, w: u32, h: u32) -> Result<Self> {
// SAFETY: every call is a `?`-checked D3D11 method on the live `device` borrow, over
// fully-initialized stack descriptors and live `Option` out-params; `compile_shader` receives
// `s!()` literals (its contract). Each created COM interface owns its own reference, and no
@@ -458,7 +467,7 @@ impl HdrP010Converter {
/// Called ONCE PER OUT-RING SLOT by the owner of the P010 textures, not per frame — see
/// [`Self::convert`]. Fails when the driver rejects a planar RTV, which is the one hard
/// requirement of this whole path (a D3D11.3+ runtime plus driver support).
pub(crate) unsafe fn plane_rtv(
pub(crate) fn plane_rtv(
device: &ID3D11Device,
dst: &ID3D11Texture2D,
format: DXGI_FORMAT,
@@ -498,7 +507,7 @@ impl HdrP010Converter {
/// ring slot's keyed-mutex hold, i.e. time the DRIVER spent blocked on that slot. Both are
/// lifetime-of-mode facts: the views belong to the out-ring slot (built in `ensure_out_ring`),
/// the buffer to this converter, which is already rebuilt on every mode change.
pub(crate) unsafe fn convert(
pub(crate) fn convert(
&self,
ctx: &ID3D11DeviceContext,
src_srv: &ID3D11ShaderResourceView,
@@ -693,7 +702,7 @@ pub(crate) struct BgraToYuvPlanes {
}
impl BgraToYuvPlanes {
pub(crate) unsafe fn new(device: &ID3D11Device, hdr: bool, chroma444: bool) -> Result<Self> {
pub(crate) fn new(device: &ID3D11Device, hdr: bool, chroma444: bool) -> Result<Self> {
// SAFETY: as `HdrP010Converter::new` — `?`-checked D3D11 shader creation on the live
// `device` borrow, with `s!()` literals into `compile_shader` and live out-params.
unsafe {
@@ -731,7 +740,7 @@ impl BgraToYuvPlanes {
/// texture) + `cbcr_rtv` (half- or full-res CbCr texture per the constructed mode). Two opaque
/// passes; `w`/`h` are the full luma dims (even for 4:2:0).
#[allow(clippy::too_many_arguments)]
pub(crate) unsafe fn convert(
pub(crate) fn convert(
&self,
ctx: &ID3D11DeviceContext,
src_srv: &ID3D11ShaderResourceView,
@@ -828,7 +837,7 @@ impl VideoConverter {
/// linear (the HDR ring, used by a PyroWave session that tone-maps the HDR desktop down to the
/// 8-bit wavelet stream). The output is always studio-range BT.709 NV12 — the P010/BT.2020 HDR
/// path is [`HdrP010Converter`]'s job, never this one.
pub(crate) unsafe fn new(
pub(crate) fn new(
device: &ID3D11Device,
context: &ID3D11DeviceContext,
width: u32,
@@ -897,11 +906,7 @@ impl VideoConverter {
/// Convert `input` (BGRA, or scRGB FP16 for a converter built with `scrgb_input`) → `output`
/// (NV12, BT.709 studio-range — see the type doc: never P010) on the video engine. Views are
/// created per call (cheap relative to the Blt) so the input texture can vary frame to frame.
pub(crate) unsafe fn convert(
&self,
input: &ID3D11Texture2D,
output: &ID3D11Texture2D,
) -> Result<()> {
pub(crate) fn convert(&self, input: &ID3D11Texture2D, output: &ID3D11Texture2D) -> Result<()> {
// SAFETY: both view creations are `?`-checked calls on `self.vdev` with fully-initialized
// stack descriptors and live out-params. `stream.pInputSurface` is a `ManuallyDrop` of the
// input view just created: `VideoProcessorBlt` only BORROWS it (a COM in-param never transfers
crates/pf-capture/src/windows/idd_push.rs
+18 -22
View File
@@ -648,11 +648,7 @@ impl IddPushCapturer {
// nothing downstream could detect the mismatch, because every field it would compare against
// had already been moved. Nothing below this line fails.
let fmt = Self::ring_format_for(new_display_hdr);
// SAFETY: `create_ring_slots` is an `unsafe fn` (it makes D3D11/DXGI COM calls); we pass a live
// borrow of `self.device` (the capturer's own device, on which the slots are created) plus plain
// `u32`/`DXGI_FORMAT` values, and `?` propagates any failure before the slots are used. Every
// returned slot's texture + keyed mutex belongs to that same `self.device`.
let new_slots = unsafe { Self::create_ring_slots(&self.device, new_w, new_h, fmt)? };
let new_slots = Self::create_ring_slots(&self.device, new_w, new_h, fmt)?;
self.display_hdr = new_display_hdr;
self.width = new_w;
self.height = new_h;
@@ -969,11 +965,11 @@ impl IddPushCapturer {
/// mid-session mode swap.
fn ensure_pyro_conv(&mut self) -> Result<()> {
if self.pyro_conv.is_none() {
// SAFETY: `BgraToYuvPlanes::new` compiles D3D11 shaders on `self.device`; `?` propagates
// failure before it is stored.
self.pyro_conv = Some(unsafe {
BgraToYuvPlanes::new(&self.device, self.display_hdr, self.want_444)?
});
self.pyro_conv = Some(BgraToYuvPlanes::new(
&self.device,
self.display_hdr,
self.want_444,
)?);
}
Ok(())
}
@@ -984,22 +980,22 @@ impl IddPushCapturer {
fn ensure_converter(&mut self) -> Result<()> {
if self.display_hdr {
if self.hdr_p010_conv.is_none() {
// SAFETY: `HdrP010Converter::new` is `unsafe` (it compiles D3D11 shaders + creates
// resources); we pass a live borrow of `self.device`, the device the converter's resources
// belong to, and `?` propagates any failure before the converter is stored.
self.hdr_p010_conv =
Some(unsafe { HdrP010Converter::new(&self.device, self.width, self.height)? });
self.hdr_p010_conv = Some(HdrP010Converter::new(
&self.device,
self.width,
self.height,
)?);
}
} else if self.want_444 {
// Full-chroma passthrough — no conversion resources to build.
} else if self.video_conv.is_none() {
// SAFETY: `VideoConverter::new` is `unsafe` (it sets up the D3D11 VIDEO processor); we pass live
// borrows of `self.device` + its immediate `self.context` (single-threaded, this thread) plus
// plain `u32` dimensions, and `?` propagates any failure before it is stored. The converter's
// resources belong to that same device/context.
self.video_conv = Some(unsafe {
VideoConverter::new(&self.device, &self.context, self.width, self.height, false)?
});
self.video_conv = Some(VideoConverter::new(
&self.device,
&self.context,
self.width,
self.height,
false,
)?);
}
Ok(())
}
crates/pf-capture/src/windows/idd_push/cursor_blend.rs
+3 -7
View File
@@ -56,7 +56,7 @@ pub(super) struct CursorBlendPass {
}
impl CursorBlendPass {
pub(super) unsafe fn new(device: &ID3D11Device) -> Result<Self> {
pub(super) fn new(device: &ID3D11Device) -> Result<Self> {
// SAFETY: `?`-checked D3D11 resource creation on the live `device` borrow, over
// fully-initialized stack descriptors and live out-params; `compile_shader` receives `s!()`
// literals (its contract).
@@ -116,11 +116,7 @@ impl CursorBlendPass {
}
/// Upload `ov`'s bitmap if its serial is new; reuse the cached SRV otherwise.
unsafe fn ensure_shape(
&mut self,
device: &ID3D11Device,
ov: &pf_frame::CursorOverlay,
) -> Result<()> {
fn ensure_shape(&mut self, device: &ID3D11Device, ov: &pf_frame::CursorOverlay) -> Result<()> {
// SAFETY: `CreateTexture2D`/`CreateShaderResourceView` are `?`-checked calls on the live
// `device` borrow. `init.pSysMem` points into `ov.rgba`, which the length check above proves
// holds at least `ov.w * ov.h * 4` bytes for the declared `SysMemPitch = ov.w * 4`, and which
@@ -170,7 +166,7 @@ impl CursorBlendPass {
/// composition) — linearize and scale to the target's SDR white. The quad is placed purely
/// via the viewport (the fullscreen-triangle VS fills whatever viewport is set), clipped by
/// the target automatically.
pub(super) unsafe fn blend(
pub(super) fn blend(
&mut self,
device: &ID3D11Device,
ctx: &ID3D11DeviceContext,
crates/pf-capture/src/windows/idd_push/open.rs
+1 -1
View File
@@ -85,7 +85,7 @@ impl IddPushCapturer {
/// to the display's composition format — FP16 in HDR, BGRA in SDR). Each is shared through an
/// UNNAMED NT handle (nothing to open by name — the sealed channel); the driver reaches it only via
/// the duplicate the [`ChannelBroker`] sends after the ring is published.
pub(super) unsafe fn create_ring_slots(
pub(super) fn create_ring_slots(
device: &ID3D11Device,
w: u32,
h: u32,