Improve scene serialized format

[djeedai]

What problem does this solve or what need does it fill?

The scene serialization (.scn.ron) is very verbose.

What solution would you like?

• Mark a few types as #[serde(transparent)]: e.g. InheritedVisibility which currently serializes as (true) but the parentheses are useless.
• Don't serialize known defaults: it's highly unlikely we'll ever change the default of Transform, and currently in RON that takes 13 lines.
• Don't serialize computed types: things like GlobalTransform are overwritten at runtime, there's no value in serializing them, it's not user data.
• Shorten type names: it's very likely there's more than 1 Transform per file for example, so if we really need the fully qualified type name, add a table in the file mapping short <-> full names, and use the short name everywhere else. This would save a lot of verbosity, and would make the file more readable (it's very unlikely someone will have a component with the same name as a Bevy one, and if they do we can still handle this with a different short name in the mapping table).

What alternative(s) have you considered?

Live with it. Not sure there's much I can do as a third party without a Bevy fix.

Additional context

Currently an empty scene with a single entity with a UI rectangle with almost all default values (so not even a useful one) serializes at 150 lines.

[alice-i-cecile]

#9538, for previous design work in this space. This is @cart's current focus.

[djeedai]

Example file

(
  resources: {},
  entities: {
    17179869205: (
      components: {
        "bevy_transform::components::transform::Transform": (
          translation: (
            x: 0.0,
            y: 0.0,
            z: 0.0,
          ),
          rotation: (
            x: 0.0,
            y: 0.0,
            z: 0.0,
            w: 1.0,
          ),
          scale: (
            x: 1.0,
            y: 1.0,
            z: 1.0,
          ),
        ),
        "bevy_transform::components::global_transform::GlobalTransform": ((
          matrix3: (
            x_axis: (
              x: 1.0,
              y: 0.0,
              z: 0.0,
            ),
            y_axis: (
              x: 0.0,
              y: 1.0,
              z: 0.0,
            ),
            z_axis: (
              x: 0.0,
              y: 0.0,
              z: 1.0,
            ),
          ),
          translation: (
            x: 0.0,
            y: 0.0,
            z: 0.0,
          ),
        )),
        "bevy_core::name::Name": "rect",
        "bevy_render::view::visibility::Visibility": Inherited,
        "bevy_render::view::visibility::InheritedVisibility": (true),
        "bevy_render::view::visibility::ViewVisibility": (true),
        "bevy_hierarchy::components::parent::Parent": (12884901914),
        "bevy_ui::ui_node::Node": (
          stack_index: 0,
          calculated_size: (
            x: 0.0,
            y: 0.0,
          ),
          outline_width: 0.0,
          outline_offset: 0.0,
          unrounded_size: (
            x: 0.0,
            y: 0.0,
          ),
        ),
        "bevy_ui::focus::FocusPolicy": Pass,
        "bevy_ui::ui_node::Style": (
          display: Flex,
          position_type: Relative,
          overflow: (
            x: Visible,
            y: Visible,
          ),
          direction: Inherit,
          left: Px(0.0),
          right: Px(0.0),
          top: Px(0.0),
          bottom: Px(0.0),
          width: Auto,
          height: Px(30.0),
          min_width: Auto,
          min_height: Auto,
          max_width: Auto,
          max_height: Auto,
          aspect_ratio: None,
          align_items: Default,
          justify_items: Default,
          align_self: Auto,
          justify_self: Auto,
          align_content: Default,
          justify_content: Default,
          margin: (
            left: Px(0.0),
            right: Px(0.0),
            top: Px(0.0),
            bottom: Px(0.0),
          ),
          padding: (
            left: Px(0.0),
            right: Px(0.0),
            top: Px(0.0),
            bottom: Px(0.0),
          ),
          border: (
            left: Px(0.0),
            right: Px(0.0),
            top: Px(0.0),
            bottom: Px(0.0),
          ),
          flex_direction: Row,
          flex_wrap: NoWrap,
          flex_grow: 0.0,
          flex_shrink: 1.0,
          flex_basis: Auto,
          row_gap: Px(0.0),
          column_gap: Px(0.0),
          grid_auto_flow: Row,
          grid_template_rows: [],
          grid_template_columns: [],
          grid_auto_rows: [],
          grid_auto_columns: [],
          grid_row: (
            start: None,
            span: Some(1),
            end: None,
          ),
          grid_column: (
            start: None,
            span: Some(1),
            end: None,
          ),
        ),
        "bevy_ui::ui_node::BackgroundColor": (Rgba(
          red: 1.0,
          green: 0.0,
          blue: 0.0,
          alpha: 1.0,
        )),
        "bevy_ui::ui_node::ZIndex": Local(0),
        "bevy_ui::ui_node::BorderColor": (Rgba(
          red: 1.0,
          green: 1.0,
          blue: 1.0,
          alpha: 1.0,
        )),
      },
    ),
  },
)

[djeedai]

#9538, for previous design work in this space. This is @cart's current focus.

Alright. But I think what's proposed there is quite different; it's "let's rewrite everything from scratch", which takes some time and seems to have a much large scope. Can't we just make the few above fixes, which for most are just a few lines here and there (e.g. adding #[serde(transparent)] is fairly simple)?

[alice-i-cecile]

Yeah, I'm generally on board with merging incremental improvements like that. We should have good serialization output regardless: it will continue to be valuable for other users of serialization (like networking).

The main challenge will be in migrating scenes between versions: it would be good to chew on automated or semi-automated tools we can use to aid this process.

[SkiFire13]

Other potential improvements:

• components is the only field of each entity, could it be flattened?
• Transform could have a specialized Serialize implementation that doesn't include all the x, y and z field names, since they can be inferred from the order. This would allow to put translation, rotation and scale on one line each, removing 13 lines for each Transform. The same could be done for BorderColor/bevy_render::color::Color and probably more, though the benefits in those cases may be smaller.

[MrGVSV]

Don't serialize known defaults: it's highly unlikely we'll ever change the default of Transform, and currently in RON that takes 13 lines.

We can't not serialize defaults since that tells the deserializer what components to insert. We can do a couple things though:

1. Have a separate entry in the entity map to include a list of default components by type name. This would be easy to implement and remove any special serialization hacks
2. Serialize them as () or some ComponentValue::Default variant. This would require some more complex deserialization and possibly affect the other non-defaults if we go the enum route. It could also prevent users from using non-self-describing formats depending on how we achieve it. And lastly, it would likely require modifying the reflection serializers rather than just the scene serializers, which might not be desirable.

Don't serialize computed types: things like GlobalTransform are overwritten at runtime, there's no value in serializing them, it's not user data.

I believe some people specifically set up their GlobalTransform to include shearing, which is why we do serialize it currently. That could have changed, though, I’m not sure.

[djeedai]

We can't not serialize defaults since that tells the deserializer what components to insert.

I think you're saying we have to serialize the component name at least? I'm saying let's not serialize the 20+ fields of Style which have a default value. That seems orthogonal no? I do agree we need the list of components, we don't really have a choice I think here.

[djeedai]

Transform could have a specialized Serialize implementation that doesn't include all the x, y and z field names, since they can be inferred from the order.

I believe this is modifying the serializing of Vec3 rather. I'm doing that in Hanabi to serialize vectors as (x,y,z) instead of { x: x, y: y, z: z }. We can achieve that with a custom Transform serializing but I feel the issue is more general.

[MrGVSV]

We can't not serialize defaults since that tells the deserializer what components to insert.

I think you're saying we have to serialize the component name at least? I'm saying let's not serialize the 20+ fields of Style which have a default value. That seems orthogonal no? I do agree we need the list of components, we don't really have a choice I think here.

Yeah that's what I’m saying. And so I’m suggesting ways we might achieve that since you can't just have a value-less key in the component map. I think the cleanest/simplest option would be to include a separate entry that lists all the defaulted components by name.

[djeedai]

Ah, because if we tell serde to not serialize default values, it will also remove the component name, is that it? Can't we mark the content of the component as "do not serialize default" without marking the component itself? Or is the issue that we don't want explicit marking because there's too many components, and you were thinking about a different way than serde attributes to achieve this @MrGVSV?

[SkiFire13]

I believe this is modifying the serializing of Vec3 rather. I'm doing that in Hanabi to serialize vectors as (x,y,z) instead of { x: x, y: y, z: z }. We can achieve that with a custom Transform serializing but I feel the issue is more general.

However Vec3 is defined in glam so you have to ask them to change its serialized format.

I think the cleanest/simplest option would be to include a separate entry that lists all the defaulted components by name.

That would still be kinda bad with Style unless #5511 is tackled first.

Another option would be putting #[serde(default, skip_serializing_if = "is_default")] on all fields, which will tell serde to avoid serializing the field if is_default(&field) is true and will use its default value if it's missing (though we should be careful to not introduce bugs here, as some fields have a default value that's different than their type's default value).

[djeedai]

Why not use #[serde(default)] on the struct components? That doesn't work for non-struct, but that gets us 90% of the main ones no?

[MrGVSV]

Ah, because if we tell serde to not serialize default values, it will also remove the component name, is that it? Can't we mark the content of the component as "do not serialize default" without marking the component itself? Or is the issue that we don't want explicit marking because there's too many components, and you were thinking about a different way than serde attributes to achieve this @MrGVSV?

How do we determine if a value is the default value? #[serde(default)] only works one way (deserialization). But even if it didn't this would mean that users would need to use serde if they want to opt into this behavior. Not the worst since that's true of many things, but not the best either.

And we don't want to do this at the reflection serializer level as that would require instantiating the default value and comparing it against each item recursively.

So to me the best solution (if we want this), would be to check it only at the component level within the bevy_scene serializer using ReflectDefault. We'd still probably incur a performance hit but it seems like the best place for it.

I think the cleanest/simplest option would be to include a separate entry that lists all the defaulted components by name.

That would still be kinda bad with Style unless #5511 is tackled first.

@SkiFire13 what do you mean? Wouldn't serializing just the type name be the same regardless of the size of the component? The only thing that would be bad would be comparing such large structs, but we'd have to do that in any solution that doesn't maintain an is_default flag or something.

[SkiFire13]

@SkiFire13 what do you mean? Wouldn't serializing just the type name be the same regardless of the size of the component?

Sorry I wasn't totally clear on the situation I had in mind. What I meant was that if even one of the fields is changed (which I would expect to happen in practice) then the entire struct will be serialized. Hence the reference to #5511, because if it was splitted then you could include only the splitted components that are set to a non-default value.

[MrGVSV]

@SkiFire13 what do you mean? Wouldn't serializing just the type name be the same regardless of the size of the component?

Sorry I wasn't totally clear on the situation I had in mind. What I meant was that even one of the fields is changed (which I would expect to happen in practice) then the entire struct will be serialized. Hence the reference to #5511, because if it was splitted then you could include only the splitted components that are set to a non-default value.

Oh makes sense! Yeah larger components would definitely suffer like that.

Another option is reflection diffing. We could diff against the default values of each component and serialize that diff, which would be a lot smaller. That was part of my initial diffing implementation but I haven't worked on it for over a year or so unfortunately.

[djeedai]

Another option is reflection diffing. We could diff against the default values of each component and serialize that diff, which would be a lot smaller. That was part of my initial diffing implementation but I haven't worked on it for over a year or so unfortunately.

I also tried myself at some point to add a Diff trait for reflection. And I think cart did, too. This seems to be coming back every once in a while.