Material colours assigned to primitives in UI are not saved correctly

[joshnewans]

Environment

• OS Version: Ubuntu 24.04
• Source or binary build? Binary 9.0.0

Description

• Expected behavior: Material data for primitives is saved in the world
• Actual behavior: Values are reset to grey

Steps to reproduce

1. Create a primitive (e.g. box or sphere) in the empty world.
2. Assign some colour values to the visual material
3. Save the world
4. Look at the SDF and note that the values seem to be default grey and not the colours you set
5. Load up your SDF and look at your sad grey world. 😥

Output

World before saving:

Snippet from saved SDF:

        <visual name='sphere_visual'>
          <geometry>
            <sphere>
              <radius>0.5</radius>
            </sphere>
          </geometry>
          <material>
            <ambient>0.300000012 0.300000012 0.300000012 1</ambient>
            <diffuse>0.699999988 0.699999988 0.699999988 1</diffuse>
            <specular>1 1 1 1</specular>
          </material>
        </visual>

[azeey]

Ah yes, the materials are just being copied from the original SDF.

gz-sim/src/SdfGenerator.cc

Lines 450 to 460 in c72be04

	// Update sdf based on current components. Here are the list of components
	// to be updated:
	// - Name
	// - Pose
	// - Static
	// - SelfCollide
	// This list is to be updated as other components become updateable during
	// simulation
	auto *nameComp = _ecm.Component<components::Name>(_entity);
	_elem->GetAttribute("name")->Set(nameComp->Data());

We need to add logic there to update the generated SDF from the components::Material component.

@joshnewans would you be interested in helping out? Thanks for the other PR you created btw.

[joshnewans]

Thanks yes I'd tracked it to that spot and then got a bit stuck but I'm happy to have a crack at it when I get a chance.

[azeey]

Awesome! I'll assign the task to you then for tracking purposes. Let us know if you have any questions.

[joshnewans]

Ok I had a look at things and have some questions already. First I noted that I think I need to dive into the updateLinkElement function since the visual data is attached to the link.

Once in there I added the following snippet (not sure about code style yet, just trying to get it to run).

    auto visuals = gz::sim::Link(_entity).Visuals(_ecm); // Looks to be the same as _ecm.EntitiesByComponents(_entity,components::Visual());

    for (const auto &v : visuals)
    {
      auto *matComp = _ecm.Component<components::Material>(v);
      auto matElem = _elem->GetElement("visual")->GetElement("material");

      auto diffuseElem = matElem->GetElement("diffuse");
      diffuseElem->Set(matComp->Data().Diffuse());
      // Todo - Repeat for specular, ambient, and emmissive as they are all configurable from the GUI

      gzdbg << "Link " << nameComp->Data() << " has visual element with diffuse " << matComp->Data().Diffuse() << std::endl;
    }

My understanding is that this should:

• Loop over all the visual entities in the link (any child entities that have a visual component)
• Get the material component of each visual entity
• Set the diffuse element of the SDF to the serialised version of the material component's diffuse (to be repeated for other characteristics)

(2025-01-28 20:12:32.481) [debug] [SdfGenerator.cc:587] Link box_link has visual element with diffuse 0.7 0.7 0.7 1
(2025-01-28 20:12:32.485) [debug] [SdfGenerator.cc:587] Link sphere_link has visual element with diffuse 0.7 0.7 0.7 1

My issue is that it seems like before I even get to worrying about saving it into the SDF, just printing the live diffuse value out of the material component seems to be incorrect?

My secondary issue/question is that it seems a little onerous to do each of these characteristics manually but perhaps that is preferred since there are many more in the spec that aren't in the GUI? It seems like someone has ran into this before, and there is a class called MaterialParser, and the Material component class seems to have a serialiser defined but no << overload which I suppose is why it can't automatically be serialised? I'm happy to do them individually, I just thought I'd check I wasn't missing something.

Thanks

[azeey]

I tried your code and was also stumped for a while. I updated one of the systems (SceneBroadcaster) to print the diffuse values for visuals on every step and noticed that the values weren't changing when I updated the color on the GUI. Then it dawned on me that there are the RenderUtil class, which is used by plugins in both the GUI and the Server, was not initialized by any plugin in the Server. This is because that only happens if we need rendering on the Server, I.e, for camera/lidar sensors. Since the Sensors system is not instantiated in the world you and I were trying, there was nothing that was updating the components::Material component on the visuals. Testing the same code in the camera_sensor.sdf world shows the expected result.

So this is going to be a bit more involved than I had originally thought. We'll need think a little bit more about how to extract the logic that updates the Material component out of RenderUtil so that the server is in sync with the GUI. cc @iche033

[joshnewans]

Ooh that's sneaky. I did come across the RenderUtil stuff as I was searching for references to materials/colours/etc. in the codebase. I'll leave that one in your hands for now if that's ok.

[iche033]

So this is going to be a bit more involved than I had originally thought. We'll need think a little bit more about how to extract the logic that updates the Material component out of RenderUtil so that the server is in sync with the GUI. cc @iche033

hmm I think we'll need to process the VisualCmd, LightCmd, etc (the code responsible for updating the ECM) outside of RenderUtil in a separate system.

options:

1. Update the material properties in ECM in the user_command system. This system is actually the one responsible for creating the VisualCmd component. So the change would be that in addition to creating the VisualCmd component, it also updates the material properties in the ECM with the requested changes. The sensors system will still need to handle VisualCmd as it needs to perform rendering operations (to actually change the materials).
   • cons: this creates inconsistency with how other *Cmd are handled
2. Handle the cmds in the physics system since that's almost always loaded. Physics system should not remove the cmd component, as the sensors system will also need to process them to perform rendering operations.
   • cons: not ideal as the physics system technically it's for physics properties only. There is now an order of execution dependency.
3. Create a new system for handling these scene / visual related cmds, e.g. Scene system.
   • cons: added complexity with a new system.

None of these options are ideal. I'm currently leaning towards option 1 but can be convinced otherwise.

[joshnewans]

An extra comment that may or may not be relevant - I don't know if this has been discussed in the past or if there is a feature request but it would be great to increase the number of parameters adjustable in the GUI, two in particular I am thinking are the geometry primitives (e.g. box dimensions, sphere radius) and scale (which gets messy but can be handy for imported meshes).

I'm sure there are many considerations in the actual implementation of this, but if one of your options is more suited for generalising this problem of mapping the live parameters into the SDF that might be preferable for future-proofing?