Skip to content
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

Angle datatype stores now only radians #6916

Merged
merged 12 commits into from
Jul 17, 2024
34 changes: 21 additions & 13 deletions crates/build/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
format_path,
objects::ObjectClass,
ArrowRegistry, Docs, ElementType, GeneratedFiles, Object, ObjectField, ObjectKind, Objects,
Reporter, Type, ATTR_CPP_NO_FIELD_CTORS,
Reporter, Type, ATTR_CPP_NO_FIELD_CTORS, ATTR_CPP_PREFIX_FIELD,
};

use self::array_builder::{arrow_array_builder_type, arrow_array_builder_type_object};
Expand Down Expand Up @@ -445,7 +445,7 @@ impl QuotedObject {
.iter()
.map(|obj_field| {
let docstring = quote_field_docs(objects, obj_field);
let field_name = format_ident!("{}", obj_field.name);
let field_name = field_name_identifier(obj_field);
let field_type = quote_archetype_field_type(&mut hpp_includes, obj_field);
let field_type = if obj_field.is_nullable {
hpp_includes.insert_system("optional");
Expand Down Expand Up @@ -476,7 +476,7 @@ impl QuotedObject {
.iter()
.map(|obj_field| {
let field_type = quote_archetype_field_type(&mut hpp_includes, obj_field);
let field_ident = format_ident!("{}", obj_field.name);
let field_ident = field_name_identifier(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
(
Expand All @@ -498,7 +498,7 @@ impl QuotedObject {

// Builder methods for all optional components.
for obj_field in obj.fields.iter().filter(|field| field.is_nullable) {
let field_ident = format_ident!("{}", obj_field.name);
let field_ident = field_name_identifier(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
let method_ident = format_ident!("with_{}", obj_field.name);
Expand Down Expand Up @@ -663,7 +663,7 @@ impl QuotedObject {
objects,
&mut hpp_includes,
obj_field,
&format_ident!("{}", obj_field.name),
&field_name_identifier(obj_field),
);
quote! {
#NEWLINE_TOKEN
Expand Down Expand Up @@ -819,7 +819,7 @@ impl QuotedObject {
}
})
.chain(obj.fields.iter().map(|obj_field| {
let ident = format_ident!("{}", obj_field.name);
let ident = field_name_identifier(obj_field);
quote! {
#ident,
}
Expand Down Expand Up @@ -896,7 +896,7 @@ impl QuotedObject {
for obj_field in &obj.fields {
let snake_case_name = obj_field.snake_case_name();
let field_name = format_ident!("{}", snake_case_name);
let tag_name = format_ident!("{}", obj_field.name);
let tag_name = field_name_identifier(obj_field);

let method = if obj_field.typ == Type::Unit {
let method_name = format_ident!("is_{}", snake_case_name);
Expand Down Expand Up @@ -950,7 +950,7 @@ impl QuotedObject {
}
})
.chain(obj.fields.iter().map(|obj_field| {
let tag_ident = format_ident!("{}", obj_field.name);
let tag_ident = field_name_identifier(obj_field);
let field_ident = format_ident!("{}", obj_field.snake_case_name());

if obj_field.typ.has_default_destructor(objects) {
Expand Down Expand Up @@ -988,7 +988,7 @@ impl QuotedObject {
let mut placement_new_arms = Vec::new();
let mut trivial_memcpy_cases = Vec::new();
for obj_field in &obj.fields {
let tag_ident = format_ident!("{}", obj_field.name);
let tag_ident = field_name_identifier(obj_field);
let case = quote!(case detail::#tag_typename::#tag_ident:);

// Inferring from trivial destructability that we don't need to call the copy constructor is a little bit wonky,
Expand Down Expand Up @@ -1215,7 +1215,7 @@ impl QuotedObject {
.enumerate()
.map(|(i, obj_field)| {
let docstring = quote_field_docs(objects, obj_field);
let field_name = format_ident!("{}", obj_field.name);
let field_name = field_name_identifier(obj_field);

// We assign the arrow type index to the enum fields to make encoding simpler and faster:
let arrow_type_index = proc_macro2::Literal::usize_unsuffixed(1 + i); // 0 is reserved for `_null_markers`
Expand Down Expand Up @@ -1263,6 +1263,14 @@ impl QuotedObject {
}
}

fn field_name_identifier(obj_field: &ObjectField) -> Ident {
if obj_field.has_attr(ATTR_CPP_PREFIX_FIELD) {
format_ident!("_{}", obj_field.name)
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
} else {
format_ident!("{}", obj_field.name)
}
}

fn single_field_constructor_methods(
obj: &Object,
hpp_includes: &mut Includes,
Expand Down Expand Up @@ -1303,7 +1311,7 @@ fn add_copy_assignment_and_constructor(
objects: &Objects,
) -> Vec<Method> {
let mut methods = Vec::new();
let field_ident = format_ident!("{}", target_field.name);
let field_ident = field_name_identifier(target_field);
let param_ident = format_ident!("{}_", obj_field.name);

// We keep parameter passing for assignment & ctors simple by _always_ passing by value.
Expand Down Expand Up @@ -1551,7 +1559,7 @@ fn archetype_serialize(type_ident: &Ident, obj: &Object, hpp_includes: &mut Incl

let num_fields = quote_integer(obj.fields.len() + 1); // Plus one for the indicator.
let push_batches = obj.fields.iter().map(|field| {
let field_name = format_ident!("{}", field.name);
let field_name = field_name_identifier(field);
let field_accessor = quote!(archetype.#field_name);

let push_back = quote! {
Expand Down Expand Up @@ -1829,7 +1837,7 @@ fn quote_append_field_to_builder(
includes: &mut Includes,
objects: &Objects,
) -> TokenStream {
let field_name = format_ident!("{}", field.name);
let field_name = field_name_identifier(field);

if let Some(elem_type) = field.typ.plural_inner() {
let value_builder = format_ident!("value_builder");
Expand Down
5 changes: 3 additions & 2 deletions crates/build/re_types_builder/src/codegen/rust/serializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,12 +447,13 @@ fn quote_arrow_field_serializer(
// hence `unwrap_or_default` (unless elements_are_nullable is false)
let quoted_transparent_mapping = if inner_is_arrow_transparent {
let inner_obj = inner_obj.as_ref().unwrap();
let quoted_inner_obj_type = quote_fqname_as_type_path(&inner_obj.fqname);
let is_tuple_struct = is_tuple_struct_from_obj(inner_obj);
let quoted_member_accessor = if is_tuple_struct {
quote!(0)
} else {
quote!(#quoted_inner_obj_type)
let inner_field_name =
format_ident!("{}", inner_obj.fields[0].snake_case_name());
quote!(#inner_field_name)
};

if elements_are_nullable {
Expand Down
1 change: 1 addition & 0 deletions crates/build/re_types_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ pub const ATTR_RUST_TUPLE_STRUCT: &str = "attr.rust.tuple_struct";
pub const ATTR_RUST_GENERATE_FIELD_INFO: &str = "attr.rust.generate_field_info";

pub const ATTR_CPP_NO_FIELD_CTORS: &str = "attr.cpp.no_field_ctors";
pub const ATTR_CPP_PREFIX_FIELD: &str = "attr.cpp.prefix_field";

pub const ATTR_DOCS_UNRELEASED: &str = "attr.docs.unreleased";
pub const ATTR_DOCS_CATEGORY: &str = "attr.docs.category";
Expand Down
4 changes: 4 additions & 0 deletions crates/store/re_types/definitions/cpp/attributes.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@ namespace cpp.attributes;

/// Don't emit parameterized constructors for fields, only copy, move and default constructors.
attribute "attr.cpp.no_field_ctors";

/// Prefixes the field name with `_` in the generated C++ code.
/// This can be used to avoid name clashes with C++ keywords or methods.
attribute "attr.cpp.prefix_field";
19 changes: 10 additions & 9 deletions crates/store/re_types/definitions/rerun/datatypes/angle.fbs
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
include "arrow/attributes.fbs";
include "rust/attributes.fbs";
include "fbs/attributes.fbs";
include "cpp/attributes.fbs";

include "./float32.fbs";

namespace rerun.datatypes;

// ---

/// Angle in either radians or degrees.
union Angle (
"attr.rust.derive": "Copy, PartialEq"
/// Angle in radians.
struct Angle (
"attr.arrow.transparent",
"attr.python.aliases": "float, int",
"attr.python.array_aliases": "npt.ArrayLike, Sequence[float], Sequence[int]",
"attr.rust.derive": "Copy, Default, PartialEq, PartialOrd, bytemuck::Pod, bytemuck::Zeroable",
"attr.rust.repr": "transparent",
"attr.cpp.no_field_ctors"
) {
/// Angle in radians. One turn is equal to 2π (or τ) radians.
/// \py Only one of `degrees` or `radians` should be set.
Radians: rerun.datatypes.Float32 (transparent),

/// Angle in degrees. One turn is equal to 360 degrees.
/// \py Only one of `degrees` or `radians` should be set.
Degrees: rerun.datatypes.Float32 (transparent),
radians: float (order: 100, "attr.cpp.prefix_field");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR but I'm surprised we didn't go for double precision here 🤔

}
2 changes: 1 addition & 1 deletion crates/store/re_types/src/archetypes/boxes3d.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/store/re_types/src/archetypes/transform3d.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading