Skip to content

Commit

Permalink
Add a robust way to temporarily end Direct2D drawing. (#578)
Browse files Browse the repository at this point in the history
#569 solved the issue for the happy path already, but if one of the
failure paths of `copy_raw_pixels` would run then the happy path's
`begin_draw` method invocation would never happen.

This PR adds a simple `DrawRestarter` that ensures drawing gets
restarted as it is dropped when execution returns from
`copy_raw_pixels`. Similar to a `finally` section in other languages.
  • Loading branch information
xStrom authored Oct 23, 2024
1 parent 1f2ac7a commit 7c99058
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 2 deletions.
42 changes: 40 additions & 2 deletions piet-common/src/direct2d_back.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,11 @@ impl<'a> BitmapTarget<'a> {
fmt: ImageFormat,
buf: &mut [u8],
) -> Result<usize, piet::Error> {
self.context.end_draw()?;
// TODO: convert other formats.
if fmt != ImageFormat::RgbaPremul {
return Err(piet::Error::NotSupported);
}
let draw_restarter = self.context.end_draw_temporarily()?;
let temp_texture = self
.d3d
.create_texture(self.width as u32, self.height as u32, TextureMode::Read)
Expand Down Expand Up @@ -204,7 +204,7 @@ impl<'a> BitmapTarget<'a> {
}
}

self.context.begin_draw();
drop(draw_restarter); // Make sure the restarter lives until this point for the happy path
Ok(size)
}

Expand Down Expand Up @@ -264,4 +264,42 @@ mod tests {
std::mem::drop(piet);
target.to_image_buf(ImageFormat::RgbaPremul).unwrap();
}

#[test]
fn copy_raw_pixels_and_continue() {
let mut device = Device::new().unwrap();
let mut bitmap = device.bitmap_target(64, 64, 1.0).unwrap();
{
let mut rc = bitmap.render_context();
let rect = Rect::new(4.0, 4.0, 8.0, 8.0);
rc.fill(rect, &Color::RED);
rc.finish().unwrap();
}
let mut data = vec![0; 64 * 64 * 4];
if !matches!(
bitmap.copy_raw_pixels(ImageFormat::RgbaSeparate, &mut data),
Err(Error::NotSupported)
) {
panic!("Expected image format to not be supported, but something else happened.")
}
let mut invalid_data = vec![0; 1];
if !matches!(
bitmap.copy_raw_pixels(ImageFormat::RgbaPremul, &mut invalid_data),
Err(Error::InvalidInput)
) {
panic!("Expected invalid input error, but something else happened.")
}
bitmap
.copy_raw_pixels(ImageFormat::RgbaPremul, &mut data)
.unwrap();
{
let mut rc = bitmap.render_context();
let rect = Rect::new(6.0, 6.0, 10.0, 10.0);
rc.fill(rect, &Color::GREEN);
rc.finish().unwrap();
}
bitmap
.copy_raw_pixels(ImageFormat::RgbaPremul, &mut data)
.unwrap();
}
}
21 changes: 21 additions & 0 deletions piet-direct2d/src/d2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,21 @@ pub struct Effect(ComPtr<ID2D1Effect>);
// piet-common direct2d_back, but the use cases are somewhat different.
pub struct BitmapRenderTarget(ComPtr<ID2D1BitmapRenderTarget>);

/// Restarts drawing when dropped.
///
/// Specifically, this struct's `Drop` implementation calls [`DeviceContext::begin_draw`].
///
/// This is useful when you need to restart drawing but have multiple return paths.
pub struct DrawRestarter<'a> {
context: &'a mut DeviceContext,
}

impl<'a> Drop for DrawRestarter<'a> {
fn drop(&mut self) {
self.context.begin_draw();
}
}

impl From<HRESULT> for Error {
fn from(hr: HRESULT) -> Error {
Error::WinapiError(hr)
Expand Down Expand Up @@ -463,6 +478,12 @@ impl DeviceContext {
}
}

/// End drawing and return a [`DrawRestarter`] which will restart drawing when dropped.
pub fn end_draw_temporarily(&mut self) -> Result<DrawRestarter, Error> {
self.end_draw()?;
Ok(DrawRestarter { context: self })
}

/// Clip axis aligned clip
///
/// Currently this should be for clipping just RenderContext::clear(). If
Expand Down

0 comments on commit 7c99058

Please sign in to comment.