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

[3.x] added optional cropping for texture atlas importer #52652

Merged

Conversation

boruok
Copy link
Contributor

@boruok boruok commented Sep 14, 2021

added optional region cropping to Texture Atlas importer. See godotengine/godot-proposals#1661

this is my second try of #52564, sorry, first ever PR. Let me know if did anything wrong.

@boruok boruok requested a review from a team as a code owner September 14, 2021 04:41
@@ -71,6 +71,7 @@ String ResourceImporterTextureAtlas::get_preset_name(int p_idx) const {
void ResourceImporterTextureAtlas::get_import_options(List<ImportOption> *r_options, int p_preset) const {
r_options->push_back(ImportOption(PropertyInfo(Variant::STRING, "atlas_file", PROPERTY_HINT_SAVE_FILE, "*.png"), ""));
r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "import_mode", PROPERTY_HINT_ENUM, "Region,Mesh2D"), 0));
r_options->push_back(ImportOption(PropertyInfo(Variant::BOOL, "cropped"), false));
Copy link
Member

Choose a reason for hiding this comment

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

Would it be clearer if the option was named crop_to_region or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@boruok boruok force-pushed the optional-crop-for-texture-atlas-importer branch from e7dbea2 to 9913af9 Compare September 14, 2021 12:26
@akien-mga akien-mga changed the title added optional cropping for texture atlas importer [3.x] added optional cropping for texture atlas importer Sep 14, 2021
@akien-mga akien-mga requested a review from a team September 14, 2021 12:57
@akien-mga
Copy link
Member

Code looks good to me, I just need to get the feature itself validated by import folks and we can merge.

This should probably have a master version too.

@fire
Copy link
Member

fire commented Oct 5, 2021

The proposal describes a real use. Will wait for the master branch pr.

@akien-mga akien-mga merged commit 5fb9a2a into godotengine:3.x Oct 5, 2021
@akien-mga
Copy link
Member

Thanks!

Merging already for 3.x to include it in 3.4 beta 6, but please remember to make a PR for master too so that both branches have the feature.

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

Successfully merging this pull request may close these issues.

4 participants