From a1dbc7a0ee026cc52ffbf711ecf0c4ad04518a0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Fri, 16 Sep 2022 17:59:05 +0200 Subject: [PATCH] gst/format: use fmt::Display whenever possible Previous implementation for the glib format new types built a `String` for the displayable value. This commit uses `fmt` mechanisms so as to limit useless allocations. --- gstreamer/src/format.rs | 52 ++++++++++++++++++++++++--------------- gstreamer/src/macros.rs | 54 +++++++++++++++++++++++++++++++---------- 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/gstreamer/src/format.rs b/gstreamer/src/format.rs index 8836924a7..9f6de7d02 100644 --- a/gstreamer/src/format.rs +++ b/gstreamer/src/format.rs @@ -1,5 +1,6 @@ // Take a look at the license at the top of the repository in the LICENSE file. +use crate::utils::Displayable; use crate::ClockTime; use crate::Format; use glib::translate::{FromGlib, GlibNoneError, IntoGlib, OptionIntoGlib, TryFromGlib}; @@ -449,8 +450,6 @@ impl Percent { impl fmt::Display for GenericFormattedValue { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use crate::utils::Displayable; - match self { Self::Undefined(val) => val.fmt(f), Self::Default(val) => val.display().fmt(f), @@ -835,19 +834,29 @@ impl FormattedValueIntrinsic for GenericFormattedValue {} impl_common_ops_for_newtype_uint!(Default, u64); impl_format_value_traits!(Default, Default, Default, u64); option_glib_newtype_from_to!(Default, u64::MAX); -option_glib_newtype_display!(Default, "(Default)"); +glib_newtype_display!( + Default, + DisplayableDefault, + DisplayableOptionDefault, + "(Default)" +); impl_common_ops_for_newtype_uint!(Bytes, u64); impl_format_value_traits!(Bytes, Bytes, Bytes, u64); option_glib_newtype_from_to!(Bytes, u64::MAX); -option_glib_newtype_display!(Bytes, "bytes"); +glib_newtype_display!(Bytes, DisplayableBytes, DisplayableOptionBytes, "bytes"); impl_format_value_traits!(ClockTime, Time, Time, u64); impl_common_ops_for_newtype_uint!(Buffers, u64); impl_format_value_traits!(Buffers, Buffers, Buffers, u64); option_glib_newtype_from_to!(Buffers, Buffers::OFFSET_NONE); -option_glib_newtype_display!(Buffers, "buffers"); +glib_newtype_display!( + Buffers, + DisplayableBuffers, + DisplayableOptionBuffers, + "buffers" +); impl FormattedValue for Undefined { type FullRange = Undefined; @@ -945,22 +954,10 @@ impl AsMut for Undefined { } } -impl fmt::Display for Undefined { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} (Undefined)", self.0) - } -} - -impl crate::utils::Displayable for Undefined { - type DisplayImpl = Undefined; - - fn display(self) -> Undefined { - self - } -} +glib_newtype_display!(Undefined, DisplayableUndefined, "(Undefined)"); impl_common_ops_for_newtype_uint!(Percent, u32); -option_glib_newtype_display!(Percent, "%"); +glib_newtype_display!(Percent, DisplayablePercent, DisplayableOptionPercent, "%"); impl FormattedValue for Option { type FullRange = Option; @@ -1364,4 +1361,21 @@ mod tests { unsafe { Option::::from_raw(Format::Time, raw_ct_none) }.into_signed(-1); assert!(signed.is_none()); } + + #[test] + fn display_new_types() { + let bytes = Bytes(42); + assert_eq!(&format!("{bytes}"), "42 bytes"); + assert_eq!(&format!("{}", bytes.display()), "42 bytes"); + + assert_eq!(&format!("{}", Some(bytes).display()), "42 bytes"); + assert_eq!(&format!("{}", Bytes::NONE.display()), "undef. bytes"); + + let gv_1 = GenericFormattedValue::Percent(Some(Percent(42))); + assert_eq!(&format!("{gv_1}"), "42 %"); + assert_eq!( + &format!("{}", GenericFormattedValue::Percent(None)), + "undef. %" + ); + } } diff --git a/gstreamer/src/macros.rs b/gstreamer/src/macros.rs index ee7907963..5dc7c875c 100644 --- a/gstreamer/src/macros.rs +++ b/gstreamer/src/macros.rs @@ -541,25 +541,53 @@ macro_rules! option_glib_newtype_from_to { }; } -macro_rules! option_glib_newtype_display { - ($name:ident, $unit:expr) => { - impl crate::utils::Displayable for Option<$name> { - type DisplayImpl = String; +// FIXME we could automatically build `$displayable_name` and +// `$displayable_option_name` if `concat_idents!` was stable. +// See: https://doc.rust-lang.org/std/macro.concat_idents.html +macro_rules! glib_newtype_display { + ($name:ident, $displayable_name:ident, $unit:expr) => { + pub struct $displayable_name($name); - fn display(self) -> String { - if let Some(val) = self { - val.display() - } else { - format!("undef. {}", $unit) - } + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + use std::fmt::Write; + + std::fmt::Display::fmt(&self.0, f)?; + f.write_char(' ')?; + f.write_str($unit) } } impl crate::utils::Displayable for $name { - type DisplayImpl = String; + type DisplayImpl = $name; - fn display(self) -> String { - format!("{} {}", self.0, $unit) + fn display(self) -> $name { + self + } + } + }; + + ($name:ident, $displayable_name:ident, $displayable_option_name:ident, $unit:expr) => { + glib_newtype_display!($name, $displayable_name, $unit); + + pub struct $displayable_option_name(Option<$name>); + + impl std::fmt::Display for $displayable_option_name { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + if let Some(val) = self.0.as_ref() { + std::fmt::Display::fmt(val, f) + } else { + f.write_str("undef. ")?; + f.write_str($unit) + } + } + } + + impl crate::utils::Displayable for Option<$name> { + type DisplayImpl = $displayable_option_name; + + fn display(self) -> Self::DisplayImpl { + $displayable_option_name(self) } } };