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

Add lightmap demo #471

Merged
merged 9 commits into from
Feb 1, 2021
Merged

Add lightmap demo #471

merged 9 commits into from
Feb 1, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Dec 2, 2020

depends on:

This PR demo's the new lightmap feature added in ign-rendering. It reads a new lightmap parameter from SDF and creates a material with lightmap using the new ign-rendering APIs.

One caveat is in the sdf-to-msgs conversion functions. I didn't add a lightmap field to the material.proto file in ign-msgs as that breaks ABI. So it's temporarily stored to the header. We can consider bumping ign-msgs if this is too much of a hack.

An indoor environment model has been uploaded onto Fuel. To run the demo:

ign gazebo -v 4 lightmap.sdf

indoor_lightmap

Signed-off-by: Ian Chen [email protected]

@iche033 iche033 requested a review from chapulina as a code owner December 2, 2020 20:59
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Dec 2, 2020
@iche033 iche033 added needs upstream release Blocked by a release of an upstream library 🏢 edifice Ignition Edifice and removed 🏢 edifice Ignition Edifice labels Dec 2, 2020
@codebot
Copy link

codebot commented Dec 3, 2020

I'm not sure exactly where the problem lies, but I get this (followed by maybe a dozen similar errors) when running it:

[GUI] [Err] [SystemPaths.cc:450] File [/home/osrf/.ignition/fuel/fuel.ignitionrobotics.org/openrobotics/models/indoor_lightmap/1/meshes/indoor_lightmap.dae] resolved to path [/home/osrf/.ignition/fuel/fuel.ignitionrobotics.org/openrobotics/models/indoor_lightmap/1/meshes/indoor_lightmap.dae] but the path does not exist

The files are actually found in

/home/mquigley/.ignition/fuel/fuel.ignitionrobotics.org/OpenRobotics/models/Indoor Lightmap/1

so it seems the username is incorrect as well as the caplitalization/space-vs-underscore of the generated filename

@iche033
Copy link
Contributor Author

iche033 commented Dec 3, 2020

I'm not sure exactly where the problem lies, but I get this (followed by maybe a dozen similar errors) when running it.

oops, I've fixed the paths in model.sdf and have updated the model on Fuel

@chapulina
Copy link
Contributor

I think you also need to add light_map to ign-fuel-tools here:

https://github.com/ignitionrobotics/ign-fuel-tools/blob/ef6c20d604fd744d709d1fb7a7e8bbf7a58e5a80/src/LocalCache.cc#L643-L658

Otherwise it's not getting converted to the URL, i.e.

                    <pbr>
                        <metal>
                            <albedo_map>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Indoor Lightmap/1/files/materials/textures/Wall_Albedo.png</albedo_map>
                            <roughness_map>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Indoor Lightmap/1/files/materials/textures/Wall_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">model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png</light_map>
                            <metalness>0.0</metalness>
                        </metal>
                    </pbr>

And results in these errors:

[GUI] [Err] [SystemPaths.cc:357] Unable to find file with URI [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SystemPaths.cc:444] Could not resolve file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SceneManager.cc:547] Unable to find file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SystemPaths.cc:357] Unable to find file with URI [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SystemPaths.cc:444] Could not resolve file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SceneManager.cc:547] Unable to find file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SystemPaths.cc:357] Unable to find file with URI [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SystemPaths.cc:444] Could not resolve file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]
[GUI] [Err] [SceneManager.cc:547] Unable to find file [model://indoor_lightmap/materials/textures/LightmapEqualizeHistogram.png]

examples/worlds/lightmap.sdf Outdated Show resolved Hide resolved
examples/worlds/lightmap.sdf Outdated Show resolved Hide resolved
@@ -301,6 +301,14 @@ msgs::Material ignition::gazebo::convert(const sdf::Material &_in)
asFullPath(workflow->EnvironmentMap(), _in.FilePath()));
pbrMsg->set_emissive_map(workflow->EmissiveMap().empty() ? "" :
asFullPath(workflow->EmissiveMap(), _in.FilePath()));

// todo(anyone) add light_map to material.proto
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we ticket an issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

This was addressed on gazebosim/gz-msgs#111. We could wait until ign-gazebo5 is updated to use ign-msgs7 and use the new field here (gazebo-tooling/release-tools#362)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is unblocked now

Signed-off-by: Ian Chen <[email protected]>
@iche033
Copy link
Contributor Author

iche033 commented Dec 10, 2020

I think you also need to add light_map to ign-fuel-tools here:

ah good catch, created pull request: gazebosim/gz-fuel-tools#146

@nkoenig nkoenig changed the base branch from main to ign-gazebo5 December 23, 2020 23:34
@chapulina chapulina changed the base branch from ign-gazebo5 to main January 5, 2021 00:29
examples/worlds/lightmap.sdf Outdated Show resolved Hide resolved
@@ -218,6 +218,7 @@ TEST(Conversions, Material)
workflow.SetEmissiveMap("emissive_map.png");
workflow.SetGlossinessMap("dummy_glossiness_map.png");
workflow.SetSpecularMap("dummy_specular_map.png");
workflow.SetLightMap("light_map.png", 1u);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the light_map.png file suppose to exist?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think this is just testing that the conversion between SDF and messages works fine, without worrying about resolving the file.

src/Conversions_TEST.cc Outdated Show resolved Hide resolved
src/Conversions.cc Outdated Show resolved Hide resolved
src/Conversions.cc Outdated Show resolved Hide resolved
Signed-off-by: Ian Chen <[email protected]>
@@ -218,6 +218,7 @@ TEST(Conversions, Material)
workflow.SetEmissiveMap("emissive_map.png");
workflow.SetGlossinessMap("dummy_glossiness_map.png");
workflow.SetSpecularMap("dummy_specular_map.png");
workflow.SetLightMap("light_map.png", 1u);
Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think this is just testing that the conversion between SDF and messages works fine, without worrying about resolving the file.

src/rendering/SceneManager.cc Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 30, 2021

Codecov Report

Merging #471 (bc3e0eb) into main (55ddac7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #471   +/-   ##
=======================================
  Coverage   77.38%   77.39%           
=======================================
  Files         213      213           
  Lines       11954    11958    +4     
=======================================
+ Hits         9251     9255    +4     
  Misses       2703     2703           
Impacted Files Coverage Δ
src/Conversions.cc 82.33% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ddac7...bc3e0eb. Read the comment docs.

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Feb 1, 2021
@chapulina chapulina merged commit d719838 into main Feb 1, 2021
@chapulina chapulina deleted the lightmap branch February 1, 2021 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants