fgerlits commented on code in PR #2258: URL: https://github.com/apache/nifi-minifi-cpp/pull/2258#discussion_r4106125613
########## minifi_rust/extensions/minifi_tensor/src/utils/dimensions.rs: ########## @@ -0,0 +1,146 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use minifi_native::{GetAttribute, MinifiError}; + +/// The exact placement of an aspect-preserving resize inside a target canvas. +/// +/// `ImageToTensor` applies this when resizing, and `FilterBoundingBoxes` inverts +/// it when un-mapping model coordinates back to the original image. Both must +/// agree down to the pixel, so the arithmetic lives here and nowhere else: +/// deriving the padding from the *unrounded* scaled size instead of `new_w`/ +/// `new_h` drifts by up to half a target pixel, which is several pixels once +/// divided back through `scale`. +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) struct LetterboxGeometry { + pub(crate) scale: f32, + pub(crate) new_width: u32, + pub(crate) new_height: u32, + pub(crate) pad_x: u32, + pub(crate) pad_y: u32, +} + +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) struct Dimensions { + pub(crate) width: f32, + pub(crate) height: f32, +} + +impl Dimensions { + /// Fit `self` into `target` preserving aspect ratio, centring the result. + /// + /// Assumes both dimensions are non-zero; `ImageToTensor::schedule` rejects a + /// zero 'Target width'/'Target height', and a decoded image always has at + /// least one pixel per axis. + pub(crate) fn letterbox_into(&self, target: Dimensions) -> LetterboxGeometry { + let scale = (target.width / self.width).min(target.height / self.height); + let new_width = (self.width * scale).round().max(1.0) as u32; + let new_height = (self.height * scale).round().max(1.0) as u32; + LetterboxGeometry { + scale, + new_width, + new_height, + // Saturating: `new_*` is clamped up to 1, so it can exceed a target + // axis of 0. Callers reject that config, but wrapping here would + // turn a misconfiguration into a panic or a garbage offset. + pad_x: (target.width as u32).saturating_sub(new_width) / 2, + pad_y: (target.height as u32).saturating_sub(new_height) / 2, + } + } + + pub(crate) fn from_image(img: &image::DynamicImage) -> Self { + Self { + width: img.width() as f32, + height: img.height() as f32, + } + } + + pub(crate) fn original_from_attributes<Context: GetAttribute>( + context: &Context, + ) -> Result<Dimensions, MinifiError> { + let orig_w = context + .get_required_attribute("image.original.width")? + .parse::<f32>()?; + + let orig_h = context + .get_required_attribute("image.original.height")? + .parse::<f32>()?; + + Ok(Dimensions { + width: orig_w, + height: orig_h, + }) + } + + pub(crate) fn target_from_attributes<Context: GetAttribute>( + context: &Context, + ) -> Result<Dimensions, MinifiError> { + let orig_w = context + .get_required_attribute("image.target.width")? + .parse::<f32>()?; + + let orig_h = context + .get_required_attribute("image.target.height")? + .parse::<f32>()?; + + Ok(Dimensions { + width: orig_w, + height: orig_h, + }) + } Review Comment: very minor copy-paste issue: ```suggestion pub(crate) fn target_from_attributes<Context: GetAttribute>( context: &Context, ) -> Result<Dimensions, MinifiError> { let target_w = context .get_required_attribute("image.target.width")? .parse::<f32>()?; let target_h = context .get_required_attribute("image.target.height")? .parse::<f32>()?; Ok(Dimensions { width: target_w, height: target_h, }) } ``` ########## minifi_rust/extensions/minifi_tensor/src/utils/bounding_box.rs: ########## @@ -0,0 +1,204 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use image::{Rgb, RgbImage}; +use minifi_native::{MinifiError, PropertyConstraints, PropertySchema, PropertyType}; +use serde::{Deserialize, Serialize}; + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct BoundingBox { + pub(crate) class_id: usize, + pub(crate) confidence: f32, + pub(crate) x_min: f32, + pub(crate) y_min: f32, + pub(crate) x_max: f32, + pub(crate) y_max: f32, +} + +fn draw_thick_rect( + img: &mut RgbImage, + left: u32, + top: u32, + right: u32, + bottom: u32, + thickness: u32, + color: Rgb<u8>, +) { + let box_width = right.saturating_sub(left); + let box_height = bottom.saturating_sub(top); + + for t in 0..thickness { + if box_width > 2 * t && box_height > 2 * t { + let rect = imageproc::rect::Rect::at((left + t) as i32, (top + t) as i32) + .of_size(box_width - 2 * t, box_height - 2 * t); + imageproc::drawing::draw_hollow_rect_mut(img, rect, color); + } + } +} + +impl BoundingBox { + pub fn class_id(&self) -> usize { + self.class_id + } + pub fn confidence(&self) -> f32 { + self.confidence + } + + pub(crate) fn calculate_intersection_over_union(box1: &BoundingBox, box2: &BoundingBox) -> f32 { + let x_left = box1.x_min.max(box2.x_min); + let y_top = box1.y_min.max(box2.y_min); + let x_right = box1.x_max.min(box2.x_max); + let y_bottom = box1.y_max.min(box2.y_max); + + if x_right < x_left || y_bottom < y_top { + return 0.0; + } + + let intersection_area = (x_right - x_left) * (y_bottom - y_top); + let box1_area = (box1.x_max - box1.x_min) * (box1.y_max - box1.y_min); + let box2_area = (box2.x_max - box2.x_min) * (box2.y_max - box2.y_min); + + let divisor = box1_area + box2_area - intersection_area; + + if intersection_area == 0f32 || divisor == 0f32 { + return 0f32; + } + + intersection_area / divisor Review Comment: Extreme nitpicking, but it hurts my eyes that this variable is not called `union_area`: ```suggestion let union_area = box1_area + box2_area - intersection_area; if intersection_area == 0f32 || union_area == 0f32 { return 0f32; } intersection_area / union_area ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
