From 51d2729a3d0a9676602e7c799d9579431ae95aaa Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 28 Jul 2022 16:10:11 -0700 Subject: [PATCH 1/9] Fix drawing of cropped images Calls CoreGraphicsContext::draw_image() from CoreGraphicsContext::draw_image_area() after cropping the image. --- piet-coregraphics/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 07eaa789..7e2e7bab 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -382,8 +382,7 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { ) { if let CoreGraphicsImage::NonEmpty(image) = image { if let Some(cropped) = image.cropped(to_cgrect(src_rect)) { - // TODO: apply interpolation mode - self.ctx.draw_image(to_cgrect(dst_rect), &cropped); + self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, _interp); } } } From e2174dd14668844ed1d17ea4ef91df6ac6373b4d Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Wed, 17 Aug 2022 11:10:51 -0700 Subject: [PATCH 2/9] Fix formatting (via cargo fmt) --- piet-coregraphics/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 7e2e7bab..114af968 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -382,7 +382,7 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { ) { if let CoreGraphicsImage::NonEmpty(image) = image { if let Some(cropped) = image.cropped(to_cgrect(src_rect)) { - self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, _interp); + self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, _interp); } } } From 4f7dc7115e0ae71abf9312a9285ff880b76c1d57 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Wed, 17 Aug 2022 12:04:57 -0700 Subject: [PATCH 3/9] Rename method parameter in accordance with coding conventions --- piet-coregraphics/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 114af968..53d83d9f 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -378,11 +378,11 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { image: &Self::Image, src_rect: impl Into, dst_rect: impl Into, - _interp: InterpolationMode, + interp: InterpolationMode, ) { if let CoreGraphicsImage::NonEmpty(image) = image { if let Some(cropped) = image.cropped(to_cgrect(src_rect)) { - self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, _interp); + self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, interp); } } } From 5df4eec2dbfe26fdad0c675c489210e33daea782 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Wed, 17 Aug 2022 17:02:19 -0700 Subject: [PATCH 4/9] Modify CoreGraphicsContext & capture_image_area to work with both y-up and y-down contexts --- piet-coregraphics/src/lib.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 53d83d9f..cede33e1 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -48,6 +48,7 @@ pub struct CoreGraphicsContext<'a> { // by CTContextGetCTM. Instead we maintain our own stack, which will contain // only those transforms applied by us. transform_stack: Vec, + y_down: bool, } impl<'a> CoreGraphicsContext<'a> { @@ -64,7 +65,7 @@ impl<'a> CoreGraphicsContext<'a> { height: f64, text: Option, ) -> CoreGraphicsContext { - Self::new_impl(ctx, Some(height), text) + Self::new_impl(ctx, Some(height), text, false) } /// Create a new context with the y-origin at the bottom right corner. @@ -77,13 +78,14 @@ impl<'a> CoreGraphicsContext<'a> { ctx: &mut CGContextRef, text: Option, ) -> CoreGraphicsContext { - Self::new_impl(ctx, None, text) + Self::new_impl(ctx, None, text, true) } fn new_impl( ctx: &mut CGContextRef, height: Option, text: Option, + y_down: bool, ) -> CoreGraphicsContext { ctx.save(); if let Some(height) = height { @@ -96,6 +98,7 @@ impl<'a> CoreGraphicsContext<'a> { ctx, text, transform_stack: Vec::new(), + y_down, } } } @@ -395,7 +398,17 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { // (see [`CoreGraphicsContext::new_impl`] for details). Since the `src_rect` we receive // as parameter is in piet's coordinate system, we need to first convert it to the CG one, // as otherwise our captured image area would be wrong. - let transformation_matrix = self.ctx.get_ctm(); + + let transformation_matrix = if self.y_down { + self.ctx.get_ctm() + } else { + self.ctx.save(); + self.ctx.translate(src_rect.x0, -src_rect.y0); + let matrix = self.ctx.get_ctm(); + self.ctx.restore(); + matrix + }; + let src_cgrect = to_cgrect(src_rect).apply_transform(&transformation_matrix); if src_cgrect.size.width < 1.0 || src_cgrect.size.height < 1.0 { From 56ea574f18d37ccdab178b06a61c500e17b4ee14 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 18 Aug 2022 17:01:33 -0700 Subject: [PATCH 5/9] Harmonize y-direction --- piet-coregraphics/src/lib.rs | 106 +++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 31 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index cede33e1..aac3f703 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -49,6 +49,7 @@ pub struct CoreGraphicsContext<'a> { // only those transforms applied by us. transform_stack: Vec, y_down: bool, + height: f64, } impl<'a> CoreGraphicsContext<'a> { @@ -99,6 +100,7 @@ impl<'a> CoreGraphicsContext<'a> { text, transform_stack: Vec::new(), y_down, + height: height.unwrap_or_default(), } } } @@ -121,14 +123,28 @@ pub enum CoreGraphicsImage { /// Empty images are not supported for core-graphics, so we need a variant here to handle that /// case. Empty, - NonEmpty(CGImage), + YUp(CGImage), + YDown(CGImage), } impl CoreGraphicsImage { + fn from_cgimage_and_ydir(image: CGImage, y_down: bool) -> Self { + match y_down { + true => CoreGraphicsImage::YDown(image), + false => CoreGraphicsImage::YUp(image), + } + } fn as_ref(&self) -> Option<&CGImage> { match self { CoreGraphicsImage::Empty => None, - CoreGraphicsImage::NonEmpty(img) => Some(img), + CoreGraphicsImage::YUp(image) | CoreGraphicsImage::YDown(image) => Some(image), + } + } + pub fn copy_image(&self) -> Option { + if let CoreGraphicsImage::YUp(image) | CoreGraphicsImage::YDown(image) = self { + Some(image.clone()) + } else { + None } } } @@ -343,19 +359,29 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { should_interpolate, rendering_intent, ); - Ok(CoreGraphicsImage::NonEmpty(image)) + + Ok(CoreGraphicsImage::from_cgimage_and_ydir(image, self.y_down)) } fn draw_image( &mut self, - image: &Self::Image, + src_image: &Self::Image, rect: impl Into, interp: InterpolationMode, ) { - let image = match image.as_ref() { - Some(img) => img, - None => return, + let image_y_down: bool; + let image = match src_image { + CoreGraphicsImage::YDown(img) => { + image_y_down = true; + img + } + CoreGraphicsImage::YUp(img) => { + image_y_down = false; + img + } + CoreGraphicsImage::Empty => return, }; + self.ctx.save(); //https://developer.apple.com/documentation/coregraphics/cginterpolationquality?language=objc let quality = match interp { @@ -366,13 +392,19 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { }; self.ctx.set_interpolation_quality(quality); let rect = rect.into(); - // CGImage is drawn flipped by default; it's easier for us to handle - // this transformation if we're drawing into a rect at the origin, so - // we convert our origin into a translation of the drawing context. - self.ctx.translate(rect.min_x(), rect.max_y()); - self.ctx.scale(1.0, -1.0); - self.ctx - .draw_image(to_cgrect(rect.with_origin(Point::ZERO)), image); + + if self.y_down && !image_y_down { + // The CGImage does not need to be inverted, draw it directly to the context. + self.ctx.draw_image(to_cgrect(rect), image); + } else { + // The CGImage needs to be flipped, which we do by translating the drawing rect to be + // centered around the origin before inverting the context. + self.ctx.translate(rect.min_x(), rect.max_y()); + self.ctx.scale(1.0, -1.0); + self.ctx + .draw_image(to_cgrect(rect.with_origin(Point::ZERO)), image); + } + self.ctx.restore(); } @@ -383,9 +415,13 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { dst_rect: impl Into, interp: InterpolationMode, ) { - if let CoreGraphicsImage::NonEmpty(image) = image { + if let CoreGraphicsImage::YDown(image) = image { + if let Some(cropped) = image.cropped(to_cgrect(src_rect)) { + self.draw_image(&CoreGraphicsImage::YDown(cropped), dst_rect, interp); + } + } else if let CoreGraphicsImage::YUp(image) = image { if let Some(cropped) = image.cropped(to_cgrect(src_rect)) { - self.draw_image(&CoreGraphicsImage::NonEmpty(cropped), dst_rect, interp); + self.draw_image(&CoreGraphicsImage::YUp(cropped), dst_rect, interp); } } } @@ -398,19 +434,24 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { // (see [`CoreGraphicsContext::new_impl`] for details). Since the `src_rect` we receive // as parameter is in piet's coordinate system, we need to first convert it to the CG one, // as otherwise our captured image area would be wrong. - - let transformation_matrix = if self.y_down { - self.ctx.get_ctm() + let src_cgrect = if self.y_down { + // If the active context is y-down (Piet's default) then we can use the context's + // transformation matrix directly. + let matrix = self.ctx.get_ctm(); + to_cgrect(src_rect).apply_transform(&matrix) } else { - self.ctx.save(); - self.ctx.translate(src_rect.x0, -src_rect.y0); + // Otherwise the active context is y-up (macOS default in coregraphics), and we need to + // temporarily translate and flip the context to capture the correct area. + let y_dir_adjusted_src_rect = Rect::new( + src_rect.x0, + self.height - src_rect.y0, + src_rect.x1, + self.height - src_rect.y1, + ); let matrix = self.ctx.get_ctm(); - self.ctx.restore(); - matrix + to_cgrect(y_dir_adjusted_src_rect).apply_transform(&matrix) }; - let src_cgrect = to_cgrect(src_rect).apply_transform(&transformation_matrix); - if src_cgrect.size.width < 1.0 || src_cgrect.size.height < 1.0 { return Err(Error::InvalidInput); } @@ -426,13 +467,16 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { if src_cgrect.size.width.round() as usize == self.ctx.width() && src_cgrect.size.height.round() as usize == self.ctx.height() { - return Ok(CoreGraphicsImage::NonEmpty(full_image)); + return Ok(CoreGraphicsImage::from_cgimage_and_ydir( + full_image, + self.y_down, + )); } - full_image - .cropped(src_cgrect) - .map(CoreGraphicsImage::NonEmpty) - .ok_or(Error::InvalidInput) + match full_image.cropped(src_cgrect) { + Some(image) => Ok(CoreGraphicsImage::from_cgimage_and_ydir(image, self.y_down)), + None => Err(Error::InvalidInput), + } } fn blurred_rect(&mut self, rect: Rect, blur_radius: f64, brush: &impl IntoBrush) { @@ -470,7 +514,7 @@ impl Image for CoreGraphicsImage { // the issue would only be accuracy of the size. match self { CoreGraphicsImage::Empty => Size::new(0., 0.), - CoreGraphicsImage::NonEmpty(image) => { + CoreGraphicsImage::YDown(image) | CoreGraphicsImage::YUp(image) => { Size::new(image.width() as f64, image.height() as f64) } } From 486153f328d6b69f09b9c13acbde7601c0500394 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 18 Aug 2022 19:44:17 -0700 Subject: [PATCH 6/9] Fix clippy dead code check --- piet-coregraphics/src/lib.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index aac3f703..e1f3448e 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -134,19 +134,12 @@ impl CoreGraphicsImage { false => CoreGraphicsImage::YUp(image), } } - fn as_ref(&self) -> Option<&CGImage> { + pub fn as_ref(&self) -> Option<&CGImage> { match self { CoreGraphicsImage::Empty => None, CoreGraphicsImage::YUp(image) | CoreGraphicsImage::YDown(image) => Some(image), } } - pub fn copy_image(&self) -> Option { - if let CoreGraphicsImage::YUp(image) | CoreGraphicsImage::YDown(image) = self { - Some(image.clone()) - } else { - None - } - } } impl<'a> RenderContext for CoreGraphicsContext<'a> { From ca0f20031c72b57f865707f02bdf3eb4d01d70e3 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 18 Aug 2022 20:18:24 -0700 Subject: [PATCH 7/9] Update test case --- piet-coregraphics/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index e1f3448e..60b9c93a 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -731,8 +731,12 @@ mod tests { let copy = piet .capture_image_area(Rect::new(100.0, 100.0, 200.0, 200.0)) .unwrap(); + + let unwrapped_copy = copy.as_ref().unwrap(); + let rewrapped_copy = CoreGraphicsImage::from_cgimage_and_ydir(unwrapped_copy.clone(), true); + piet.draw_image( - ©, + &rewrapped_copy, Rect::new(0.0, 0.0, 400.0, 400.0), InterpolationMode::Bilinear, ); From f5abbb15391aa585440f6c5c0725c2251a4f432f Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 18 Aug 2022 22:34:13 -0700 Subject: [PATCH 8/9] Flush crop after capturing area of a context --- piet-coregraphics/src/lib.rs | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 60b9c93a..4304860b 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -466,9 +466,35 @@ impl<'a> RenderContext for CoreGraphicsContext<'a> { )); } - match full_image.cropped(src_cgrect) { - Some(image) => Ok(CoreGraphicsImage::from_cgimage_and_ydir(image, self.y_down)), - None => Err(Error::InvalidInput), + let cropped_image_result = full_image.cropped(src_cgrect); + if let Some(image) = cropped_image_result { + // CGImage::cropped calls CGImageCreateWithImageInRect to set the bounds of the image, + // but it does not affect the underlying image data. This causes issues when using the + // captured images if the image's width does not match the original context's row size. + // To fix this, we create a new image-sized bitmap context, paint the image to it, and + // then re-capture. This forces coregraphics to resize the image to its specified bounds. + let cropped_image_size = Size::new(src_cgrect.size.width, src_cgrect.size.height); + let cropped_image_rect = Rect::from_origin_size(Point::ZERO, cropped_image_size); + let cropped_image_context = core_graphics::context::CGContext::create_bitmap_context( + None, + cropped_image_size.width as usize, + cropped_image_size.height as usize, + 8, + 0, + &core_graphics::color_space::CGColorSpace::create_device_rgb(), + core_graphics::base::kCGImageAlphaPremultipliedLast, + ); + cropped_image_context.draw_image(to_cgrect(cropped_image_rect), &image); + let cropped_image = cropped_image_context + .create_image() + .expect("Failed to capture cropped image from resize context"); + + Ok(CoreGraphicsImage::from_cgimage_and_ydir( + cropped_image, + self.y_down, + )) + } else { + Err(Error::InvalidInput) } } From 73a44fb46cd98d0db101572be6a8e246d395c0b6 Mon Sep 17 00:00:00 2001 From: longmathemagician Date: Thu, 18 Aug 2022 23:13:48 -0700 Subject: [PATCH 9/9] Rename CoreGraphicsImage::as_ref to CoreGraphicsImage::as_cgimage so as to not conflict with as_ref trait Edit: Fix typo in commit message --- piet-coregraphics/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/piet-coregraphics/src/lib.rs b/piet-coregraphics/src/lib.rs index 4304860b..dbae7aeb 100644 --- a/piet-coregraphics/src/lib.rs +++ b/piet-coregraphics/src/lib.rs @@ -134,7 +134,7 @@ impl CoreGraphicsImage { false => CoreGraphicsImage::YUp(image), } } - pub fn as_ref(&self) -> Option<&CGImage> { + pub fn as_cgimage(&self) -> Option<&CGImage> { match self { CoreGraphicsImage::Empty => None, CoreGraphicsImage::YUp(image) | CoreGraphicsImage::YDown(image) => Some(image), @@ -758,7 +758,7 @@ mod tests { .capture_image_area(Rect::new(100.0, 100.0, 200.0, 200.0)) .unwrap(); - let unwrapped_copy = copy.as_ref().unwrap(); + let unwrapped_copy = copy.as_cgimage().unwrap(); let rewrapped_copy = CoreGraphicsImage::from_cgimage_and_ydir(unwrapped_copy.clone(), true); piet.draw_image(