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

Added support for fuel textures #342

Merged
merged 6 commits into from
Apr 26, 2021
Merged

Conversation

luca-della-vedova
Copy link
Member

New feature implementation

Implemented feature

Added support for downloading textures from URLs (tested with Fuel).

Implementation description

As we move into more realistic looking worlds we can't keep adding textures to the git repo since high quality textures can be in the order of multiple MB.
This PR introduces support for URLs in the texture_name field for walls and floors. If the field is a valid URL the floor / wall generator will download it and copy the texture, if it isn't it will try the legacy way of doing a file copy.

An alternative implementation could be using pit_crew, it would have the benefit of having better isolation between the code that downloads models and the code that builds maps (as well as actually skipping all downloads if the users wanted to), but it could be a bit trickier to implement since to my understanding pit_crew doesn't have logic to access models and download single files.

Also the workflow of "using a fuel URL for a single texture" is supported by Ignition and will make future work (adding multiple textures for PBR assets) a lot easier to implement.
For example a typical sdf for a PBR asset, the indoor lightmap model, looks like this

[...]
                        <metal>
                            <albedo_map>https://fuel.ignitionrobotics.org/1.0/openrobotics/models/indoor lightmap/1/files/materials/textures/Floor_Albedo.png</albedo_map>
                            <normal_map>https://fuel.ignitionrobotics.org/1.0/openrobotics/models/indoor lightmap/1/files/materials/textures/Floor_Normal.png</normal_map>
                            <roughness_map>https://fuel.ignitionrobotics.org/1.0/openrobotics/models/indoor lightmap/1/files/materials/textures/Floor_Roughness.png</roughness_map>
                            <environment_map>https://fuel.ignitionrobotics.org/1.0/openrobotics/models/indoor lightmap/1/files/materials/textures/IndoorCubemap.dds</environment_map>
                            <light_map uv_set="1">https://fuel.ignitionrobotics.org/1.0/openrobotics/models/indoor lightmap/1/files/materials/textures/LightmapEqualizeHistogram.png</light_map>
                            <metalness>0.0</metalness>
                        </metal>

Try it!

To test the PR, just open a project in traffic_editor and assign a fuel texture URL to the texture name, for example:

image

Then run the demo

image

And enjoy the high resolution textures!

Next steps

To get the best possible results for Ignition we will need to implement fields for all the other PBR maps (such as normal, roughness, metal, environment). They can be implemented through a similar workflow by adding more fields to traffic_editor for the different texture maps.

Signed-off-by: Luca Della Vedova <[email protected]>
Signed-off-by: Luca Della Vedova <[email protected]>
@codebot
Copy link
Contributor

codebot commented Apr 24, 2021

I wonder if we should/could somehow have a "material name" type of scheme, where (by default) it would use the texture name not only for the color/albedo, but if it finds the other maps (normal, roughness, metal, environment) exist with the same name, it will use them by default at the same scale. That seems like it would be the 99% use case, where if you get one, you get them all.

@codebot
Copy link
Contributor

codebot commented Apr 24, 2021

That could be a separate PR though; this initial improvement for high-resolution color textures is awesome!

@luca-della-vedova
Copy link
Member Author

I wonder if we should/could somehow have a "material name" type of scheme, where (by default) it would use the texture name not only for the color/albedo, but if it finds the other maps (normal, roughness, metal, environment) exist with the same name, it will use them by default at the same scale. That seems like it would be the 99% use case, where if you get one, you get them all.

Agreed, I think that is the most common use case, I was thinking of something along the lines of:

  • User sets a texture (i.e. the diffuse texture)
  • There is still "add parameter" fields for each other texture but if a texture was specified it will come pre-populated. For example if there was a Floor_Diffuse the roughness map parameter's default will be Floor_Rough
  • Users can edit the default if needed.

This should cover some of the edge cases, such as:

  • Users having some textures that don't follow the naming convention we expect (for example some people call roughness maps Asset_Rough, other Asset_Roughness). They can edit the default
  • Some assets don't have all the maps, for example a floor wouldn't have a Metal map but a metal object (i.e. a wheelchair) would.
  • Environment maps are a bit special since they represent the environment the object is in and hence are not object specific, so I expect they might be called something like Clinic_Environment rather than Wheelchair_Environment.

Anyway all of those are Ignition specific, while this PR is a rendering improvement that can be applied to both Ignition and Gazebo classic

Signed-off-by: Luca Della Vedova <[email protected]>
@youliangtan
Copy link
Member

Nice feature to have! Tested it, works well. Can we also check if the texture_file exists before downloading it, cuz i notice that it will make multiple redundant downloads if we specify it multiple times.

Also, one small thing that i noticed, the scaling for the wall texture is slightly off.
Screenshot from 2021-04-26 14-06-25

Signed-off-by: Luca Della Vedova <[email protected]>
@luca-della-vedova
Copy link
Member Author

luca-della-vedova commented Apr 26, 2021

Nice feature to have! Tested it, works well. Can we also check if the texture_file exists before downloading it, cuz i notice that it will make multiple redundant downloads if we specify it multiple times.

Done!

Also, one small thing that i noticed, the scaling for the wall texture is slightly off.

Yea this is documented in #338, we need to add support for texture scaling for walls. For now walls expect a 2.5m x 1m texture so if a square one is provided (like the floor tiles you are using) they will be stretched vertically by a factor of 2.5.
I'll add it as a TODO to #343

Copy link
Member

@youliangtan youliangtan left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants