open-rmf / rmf_site

Experimental visualizer for dense buildings in RMF
32 stars 13 forks source link

Migrate relative paths does not work as expected #213

Open luca-della-vedova opened 5 months ago

luca-della-vedova commented 5 months ago

When loading a file with relative paths (i.e. drawings) and saving to a different path, the path migration does not quite seem to do what it's supposed to.

I added the following path to rmf_site_format/asset_source.rs to show what's happening:

diff --git a/rmf_site_format/src/asset_source.rs b/rmf_site_format/src/asset_source.rs
index a02df55..e58901e 100644
--- a/rmf_site_format/src/asset_source.rs
+++ b/rmf_site_format/src/asset_source.rs
@@ -69,6 +69,7 @@ impl AssetSource {
                     new_reference_path,
                 )
                 .ok_or(())?;
+                dbg!(&old_reference_path, &new_reference_path, &new_path);
                 *asset_path = new_path.to_str().ok_or(())?.to_owned();
             }
         }

Then ran:

cargo run --release -- ../roscon_workshop/roscon_maps/maps/workshop/workshop.building.yaml

And saved to the same folder. The printout was the following:

Changing path for ["conference_room_b1.png"]
[rmf_site_format/src/asset_source.rs:72:17] &old_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.building.yaml"
[rmf_site_format/src/asset_source.rs:72:17] &new_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.site.ron"
[rmf_site_format/src/asset_source.rs:72:17] &new_path = "../conference_room_b1.png"
Changing path for ["scans/sim_hall.png"]
[rmf_site_format/src/asset_source.rs:72:17] &old_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.building.yaml"
[rmf_site_format/src/asset_source.rs:72:17] &new_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.site.ron"
[rmf_site_format/src/asset_source.rs:72:17] &new_path = "../scans/sim_hall.png"
Changing path for ["conference_room_b1_balcony.png"]
[rmf_site_format/src/asset_source.rs:72:17] &old_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.building.yaml"
[rmf_site_format/src/asset_source.rs:72:17] &new_reference_path = "../roscon_workshop/roscon_maps/maps/workshop/workshop.site.ron"
[rmf_site_format/src/asset_source.rs:72:17] &new_path = "../conference_room_b1_balcony.png"
2024-04-05T03:03:22.441415Z  INFO librmf_site_editor::site::save: Save successful
2024-04-05T03:03:22.480000Z ERROR librmf_site_editor::site::drawing: Failed loading drawing "file://../conference_room_b1.png"
2024-04-05T03:03:22.480041Z ERROR librmf_site_editor::site::drawing: Failed loading drawing "file://../scans/sim_hall.png"
2024-04-05T03:03:22.480049Z ERROR librmf_site_editor::site::drawing: Failed loading drawing "file://../conference_room_b1_balcony.png"

Note how there is an additional ../ in front of the path, and for this reason the newly saved file is unable to load the drawings. I believe, based on the old and new reference paths, the migrated path should not contain the extra ../ since they are in the same folder.

mxgrey commented 5 months ago

Is this fixed by #214? Your comment in the PR is that it mostly addresses the issue, but it's not clear to me what else needs to be addressed.

luca-della-vedova commented 5 months ago

The missing part is that the combination of relative and absolute paths does not work.

This means that if a user was to open a project when using a relative path, for example through:

cargo run --release -- path_to_file.site.ron

Then save it from the UI to a different file, migration between the relative and the absolute path fails and the new paths won't work anymore. Everything else (I believe) works as expected.