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

Support Author-namedspaced Thumbnails and Revamp building_map_generator #132

Merged

Conversation

methylDragon
Copy link
Collaborator

Unify building_map_generator

  • This PR unifies all building_map_generators into a single one and implements argparse command line argument handling.
  • Additionally, the gazebo generator will call pit_crew to obtain missing models automatically when generating .world files.

Support Author-namespaced Thumbnails

  • Additionally, the thumbnail generators have been revamped to support the new author-namedspaced thumbnail directory structure as shown in Migrate thumbnails from traffic_editor repo traffic_editor_assets#2
  • Lastly, output_dir for crop.py has been set by default to the ~/.traffic_editor/assets/thumbnails/images/cropped/ directory, for parity with the automatic clone on build of traffic_editor of traffic_editor_assets

Notes

  • A new package dependency for traffic_editor has been added. Specifically for building_map_tools, in order to ensure that traffic_editor has access to the pit_crew dependency.

  • Request for test: I can't test the traffic_editor thumbnail generation pipeline due to having a different version of OpenCV (4.0.0), could someone test it out?

```

Similarly, the folder `images/white/` will have been populated when the script finishes.

The contours of the models are then extracted from the green-screened images using OpenCV to generate a transparency mask, used for cropping the white-background images, so that it stays centered and is as small as possible. This can be done by calling,

```bash
./scripts/crop.py -m test/model_list.yaml -g images/green -w images/white -o images/cropped
./scripts/crop.py test/model_list.yaml images/cropped -g images/green -w images/white
Copy link
Member

Choose a reason for hiding this comment

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

the -o argument is still required in the source code, were you intending to remove it?

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for all the renovation! The generation pipeline works really well, and the thumbnail generation is doing great too :D
Minor comment regarding the instructions of crop.py after testing

@methylDragon
Copy link
Collaborator Author

Oh, whoops!

Output dir should be optional, I'll update the readme accordingly.

@methylDragon
Copy link
Collaborator Author

methylDragon commented May 27, 2020

Output for doesn't look to be required in the source code eh :O

Only in building_map_generator, but not copy.py?

@methylDragon
Copy link
Collaborator Author

Yep! When you have - and -- in argparse, it specifies an optional argument

@methylDragon
Copy link
Collaborator Author

methylDragon commented May 27, 2020

Or do you mean the model.yaml?

@methylDragon
Copy link
Collaborator Author

image

@methylDragon
Copy link
Collaborator Author

methylDragon commented May 27, 2020

Clarification:
Specifying an output directory is optional, if you want to specify one, -o or --output_dir should be specified

I missed that in the README when I was editing it, 8dc9a5f fixes it

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

LGTM!

@methylDragon
Copy link
Collaborator Author

👍 I can't merge it due to no write access though haha

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.

None yet

2 participants