Skip to content

Remove optionality from _AttachableImageWrapper.wrappedValue on Windows. #1258

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

Merged
merged 1 commit into from
Aug 11, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,14 @@ public protocol _AttachableByAddressAsIWICBitmap {
/// - Returns: A copy of `imageAddress`, or `imageAddress` if this type does
/// not support a copying operation.
///
/// - Throws: Any error that prevented copying the value at `imageAddress`.
///
/// The testing library uses this function to take ownership of image
/// resources that test authors pass to it. If possible, make a copy of or add
/// a reference to the value at `imageAddress`. If this type does not support
/// making copies, return `imageAddress` verbatim.
///
/// This function is not part of the public interface of the testing library.
/// It may be removed in a future update.
static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self>
static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self>

/// Manually deinitialize any resources at the given address.
///
Expand Down Expand Up @@ -135,16 +133,14 @@ public protocol AttachableAsIWICBitmap {
/// - Returns: A copy of `self`, or `self` if this type does not support a
/// copying operation.
///
/// - Throws: Any error that prevented copying this value.
///
/// The testing library uses this function to take ownership of image
/// resources that test authors pass to it. If possible, make a copy of or add
/// a reference to `self`. If this type does not support making copies, return
/// `self` verbatim.
///
/// This function is not part of the public interface of the testing library.
/// It may be removed in a future update.
func _copyAttachableValue() throws -> Self
func _copyAttachableValue() -> Self

/// Manually deinitialize any resources associated with this image.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ extension HBITMAP__: _AttachableByAddressAsIWICBitmap {
return bitmap
}

public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
guard let result = CopyImage(imageAddress, UINT(IMAGE_BITMAP), 0, 0, 0)?.assumingMemoryBound(to: Self.self) else {
throw Win32Error(rawValue: GetLastError())
}
return result
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
// The only reasonable failure mode for `CopyImage()` is allocation failure,
// and Swift treats allocation failures as fatal. Hence, we do not check for
// `nil` on return.
CopyImage(imageAddress, UINT(IMAGE_BITMAP), 0, 0, 0).assumingMemoryBound(to: Self.self)
}

public static func _deinitializeAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ extension HICON__: _AttachableByAddressAsIWICBitmap {
return bitmap
}

public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
guard let result = CopyIcon(imageAddress) else {
throw Win32Error(rawValue: GetLastError())
}
return result
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
// The only reasonable failure mode for `CopyIcon()` is allocation failure,
// and Swift treats allocation failures as fatal. Hence, we do not check for
// `nil` on return.
CopyIcon(imageAddress)
}

public static func _deinitializeAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension IWICBitmap: _AttachableByAddressAsIWICBitmap {
return imageAddress
}

public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
_ = imageAddress.pointee.lpVtbl.pointee.AddRef(imageAddress)
return imageAddress
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@

internal import WinSDK

/// A type describing errors that can be thrown by WIC or COM while attaching an
/// image.
/// A type representing an error that can occur when attaching an image.
enum ImageAttachmentError: Error {
/// A call to `QueryInterface()` failed.
case queryInterfaceFailed(Any.Type, HRESULT)

/// The testing library failed to create a WIC object.
/// The testing library failed to create a COM object.
case comObjectCreationFailed(Any.Type, HRESULT)

/// An image could not be written.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ extension UnsafeMutablePointer: AttachableAsIWICBitmap where Pointee: _Attachabl
try Pointee._copyAttachableIWICBitmap(from: self, using: factory)
}

public func _copyAttachableValue() throws -> Self {
try Pointee._copyAttachableValue(at: self)
public func _copyAttachableValue() -> Self {
Pointee._copyAttachableValue(at: self)
}

public consuming func _deinitializeAttachableValue() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,26 @@ internal import WinSDK
@_spi(Experimental)
public final class _AttachableImageWrapper<Image>: Sendable where Image: AttachableAsIWICBitmap {
/// The underlying image.
nonisolated(unsafe) let image: Result<Image, any Error>
nonisolated(unsafe) let image: Image

/// The image format to use when encoding the represented image.
let imageFormat: AttachableImageFormat?

init(image: borrowing Image, imageFormat: AttachableImageFormat?) {
self.image = Result { [image = copy image] in
try image._copyAttachableValue()
}
self.image = image._copyAttachableValue()
self.imageFormat = imageFormat
}

deinit {
if let image = try? image.get() {
image._deinitializeAttachableValue()
}
image._deinitializeAttachableValue()
}
}

// MARK: -

extension _AttachableImageWrapper: AttachableWrapper {
public var wrappedValue: Image? {
try? image.get()
public var wrappedValue: Image {
image
}

public func withUnsafeBytes<R>(for attachment: borrowing Attachment<_AttachableImageWrapper>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
Expand All @@ -74,7 +70,7 @@ extension _AttachableImageWrapper: AttachableWrapper {
}

// Create the bitmap and downcast it to an IWICBitmapSource for later use.
let bitmap = try image.get().copyAttachableIWICBitmapSource(using: factory)
let bitmap = try image.copyAttachableIWICBitmapSource(using: factory)
defer {
_ = bitmap.pointee.lpVtbl.pointee.Release(bitmap)
}
Expand Down