Allow configuring the maximum width for atlas import

[AThousandShips]

Unsure about the best way of solving this, another option would be to improve the general size picking for the packer, but that becomes much more complex and I suspect it would have issues with optimization to adjust both axes

This feels like a decent enough basic improvement, and the current cap of 2048 is quite restrictive in some cases, but keeping it at that level by default to avoid unnecessary updates to existing projects

Will see about adding a warning in the importer to warn when the texture is above a certain size, and suggesting modifying this setting accordingly

Todo:

• Improve the documentation phrasing
• Consider a warning

As discussed in:

• Images with big AtlasTextures are broken in systems with AMD GPUs #87119

[AThousandShips]

Added a warning, checking if the result is more than twice as high as it is wide, this happens when the texture can't be fit in the maximum width easily, as the packer stops when the height is no larger than twice the width, or when it reaches the maximum width

[AThousandShips]

I set the minimum size to 128, I think that makes sense as a lower bound, we can go with the 1 value but it's not gonna be very useful

[AThousandShips]

An alternative method would be to uncap this value, or use a very high limit, and then have a hard limit as a setting, as the height value will grow as large as it can anyway, regardless of the width, so having it set to 2048 doesn't limit the height to anything reasonable for any platform

In that case we would have a limit that fails or similar when surpassed, but that would require a bit of a larger rework and evaluation of the whole code, so I feel that's for a larger discussion, it would also require handling errors here etc., but will look into this as well as a solution

This proposed change would break existing projects depending on context so I think it'd be a different option to look into, I would consider considering this fix more of a bug fix and cherry picking it at least to 4.2 and then adding a new system

[LakshayaG73]

I was able to use this PR to generate new AtlasTextures, and go back to using 4.2 stable : )

[LakshayaG73]

So about this PR. I can't seem to use AtlasTextures generated via this PR, if I delete the .import folder and use 4.2. Does that cause global re-imports?

[AThousandShips]

What import folder? There isn't an import folder in 4.x, do you mean .godot?

How do you mean "use 4.2"? This isn't merged so it won't work with 4.2, you can't use the imports from this in an unmodified version

[LakshayaG73]

So this change seems to be incomplete. The atlast textures revert, even when using this modified Godot, afted deleting the .godot folder. It seems that this new width for atlastextures is not respected when you delete the .godot folder.

[AThousandShips]

Did you change the setting? What isn't respected specifically? The width is an upper limit, not the width to use directly, what other settings do you use?

[LakshayaG73]

The new atlas texture width setting is set to 8192.

The width is reverted back to the default limit of 2048 after deleting the .godot folder.

Not sure what you mean by other settings.

[AThousandShips]

Settings like texture limit, please add an MRP to test this out, I can't confirm this with these details

[LakshayaG73]

I'm not sure how I can add another MRP for this. On the current MRP for this, if you delete the .godot folder, you will see that the image gets broken again.

[AThousandShips]

Okay I couldn't confirm that, will test again later today

[AThousandShips]

Tested again, and it works fine when deleting the .godot folder, so I don't know what's going on on your end

[LakshayaG73]

You are correct. It is fine. I may have gotten the versions mixed up during re-import T.T

How can we get this reviewed and into the engine? : )

[lyuma]

This should be in the import settings in resource_importer_texture_atlas.cpp, not a project setting.

Note that default import settings are part of the project settings, in the Import Defaults tab, so doing it this way would allow configuration per-file as well as a global project default.

See how the crop_to_region property is done for an example.
You can also look at resource_importer_texture.cpp for more examples of similar settings. For example, see how it declares size_limit:

  r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "process/size_limit", PROPERTY_HINT_RANGE, "0,4096,1"), 0));

[AThousandShips]

This should be in the import settings in resource_importer_texture_atlas.cpp, not a project setting.

No it can't, I've tested, it can't be because those are specific to each texture in the atlas, not the resulting atlas, how would that work if the values are different?

Having it per-file would be confusing, and weird

The result file from the atlas doesn't know it's the result of one, and the import details from it are not available to the importer, so it can't be configured there

This is also not a thing that controls how wide the resulting texture will be, but the limit, so it doesn't need to be configured individually

[lyuma]

for (const KeyValue<String, HashMap<StringName, Variant>> &E : p_source_file_options) {

It does have access to the individual options of source files, and so you could have a policy to take the minimum of all the max_width, for example.

Having this be project-wide might work for small projects, but wouldn't it be a problem if you have multiple atlases with different requirements? What happens if one atlas contains textures larger than your max_width (maybe you have wide header banners in one atlas or something)?

The result file from the atlas doesn't know it's the result of one, and the import details from it are not available to the importer
I do remember finding this pretty confusing the last time I used atlases.

I still think that having the limit set on the per-file import settings and take the minimum should work. I'm trying to avoid creating a precedent in import code of using project settings instead of import settings.

[AThousandShips]

What happens if one atlas contains textures larger than your max_width (maybe you have wide header banners in one atlas or something)?

The same that happens now, the file becomes very tall, if it's larger on both axes it will fail as the system doesn't support that at all (or it will pick that size as the width, no matter what, don't remember)

The requirement isn't a hard one, if the file can fit in 2048 it won't use 4096 width

So I don't see there being textures that need a smaller limit, what situation would that be? Where a texture must be no more than 2048 pixels on the width, but can be any height? And will be very tall?

It does have access to the individual options of source files, and so you could have a policy to take the minimum of all the max_width, for example.

I know I have access to it 🙂 I've worked with this code before, I know what I'm doing

---

To explain here, the packer tries to fit all the textures into some width, then it accepts it if it fits so that the height is no more than twice the width, or if the width is the maximum, this controls that width, so, for example, you don't get a 16k by 2k texture

[AThousandShips]

It doesn't really make sense to put it on the individual input files because it has nothing to do with them, further it'd be frustrating and confusing if you forget to update one and the minimum value doesn't change

[LakshayaG73]

As a consumer, I can say that this setting as implemented works well.

It allows you to configure the atlas widths based on your target platforms. It only configures the maximum possible width, and does not set the actual width per atlas, which is determined by the source images of the atlas.

To me, it also doesn't make sense for it to be an import setting. You only want to change this setting on a project level, as it is only dependent on your target audience and their hardware. It shouldn't need to change within a project.

@lyuma

[AThousandShips]

If/when we merge this and can cherry pick it I am planning to also look into a broader rewrite of the atlas packer and some other components, to improve some elements of it

But they'll likely involve some changes that while not necessarily breaking compat might disrupt imports so wanting to avoid cherry picking them

Some ideas are allowing multiple resulting files to avoid large resulting files, and some theoretical packing improvements I'll look into, but it's a longer term project and wanting a simpler fix that'll solve the issue at hand

[lyuma]

To be clear, the main reason I was unhappy with this proposed project setting is because it makes it unpredictable to transfer atlas textures between projects even together with the .import file, since the importer may unexpectedly recreate the atlas with a different width (which would cause changes in version control).

I was also concerned about Calinou's comment that changing the default can affect compatibility, but I don't quite understand how this would affect compatibility. It shouldn't matter which resolution is used to generate the atlas, since TextureAtlas is only supported for usage in 2D, in which the implementation details and the UVs are not exposed to the user.

If we later want to change the default project resolution to 4096, or perhaps pick a width dynamically as needed, that would cause generated atlases to be reimported but otherwise should not affect any scenes or objects which reference that atlas. Do I understand this correctly?

[lyuma]

Approved on the basis that atlas UVs are arbitrary and not directly referenced by application, so atlas textures transferred between projects with a different max width setting should work equivalently.

[AThousandShips]

The default isn't changed by this so no compatibility effects here, but indeed it doesn't really change compatibility in either case, the default from the EditorAtlasPacker was 2048 to begin with so the default project setting matches that, so the behavior is unchanged with the current settings

Currently this also does not cause any reimport, nor does changing the setting cause a reimport on its own, you can therefore (theoretically) change the project setting as needed and reimport and then change it back, though the project setting will only affect cases that won't fit at all, i.e. elongated cases

The current packing code takes the maximum width extracted from the atlases (or height, if it's flipped, taking the shorter length) and takes the next power of two, this width is used for packing, it then stops the process if either:

• The height is no greater than 2 * current_width
• current_width is greater than the maximum width provided

So if your textures fit in 1024x2048 it won't try to pack into a larger atlas, so this only affects atlases which wouldn't originally fit into 2048x4096, where the main problem was that they'd grow massively tall, so you'd get 2048x48000, which caused rendering errors on some hardware passing the maximum texture size

And indeed the post-import location in the atlas is a local difference and isn't stored in the import file, so it doesn't affect transfer of projects between units

I'm hoping to look into a future, better solution, including allowing you to split textures between imported atlas files, that would be a bit more complex and might in turn affect output, but it'd be something to look at for compatibility going forward (imagining this as a helper tool to aid in dividing up input textures into multiple atlases as opposed to an automatic thing, as that's a complicated and tedious process, and hit and miss easily)

Thank you and sorry for any communication confusion :)

[akien-mga]

Could you squash the commits? Or if it's meaningful to keep those two changes separate, the second commit needs a more detailed commit message.

[AThousandShips]

Will squash :) was just to keep it optional

[akien-mga]

Thanks!

[AThousandShips]

Thank you!

[akien-mga]

Cherry-picked for 4.2.2.