Add direction property to AtlasTexture

[onesuchkeeper]

ref godot-proposals issue #11630

• Added enum 'Direction' as a variant with possible values: NORTH, EAST, SOUTH, WEST

• Added 'direction' property of type 'Direction' to AtlasTexture. When drawing the region specified by the AtlasTexture it will be drawn with its specified direction facing up.

It is common practice for Atlases to rotate some of their members 90 degrees to better pack them into a square for read effeciency. Because of this some of their members must be rotated back at runtime.

This will allow these rotated members to be handled at the texture scope along with the 'region' and 'margin'.

Typical sheets will only use 'North' and 'East' so the existing enum 'Orientation' could be used instead of 'Direction', but I feel handling all 4 cardinal will be better in the case an abnormal sprite sheet is made.

• Bugsquad edit, closes: Add Direction property to AtlasTexture to handle atlases with rotated members at the resource scope godot-proposals#11630

[JoNax97]

Using cardinal directions for the enum feels a bit weird. I understand the idea of using the face of a compass to indicate where the top of the element, but I think the following would be more clear:

• Name the enum rotation instead of Direction
• Have the values be 0, 90, 180, 270
• In code, name the elements something that's a valid identifier, like ROTATION_0_DEG, etc.
• If more user-friendly displayed values are desired, the editor can have a special case for this enum and display it as 0°, 90°, etc.

Alternatively, maybe rotation can be defined as a floating point value? Is there value in accommodating arbitrarily rotated elements?

[onesuchkeeper]

Using cardinal directions for the enum feels a bit weird. I understand the idea of using the face of a compass to indicate where the top of the element, but I think the following would be more clear:

• Name the enum rotation instead of Direction
• Have the values be 0, 90, 180, 270
• In code, name the elements something that's a valid identifier, like ROTATION_0_DEG, etc.
• If more user-friendly displayed values are desired, the editor can have a special case for this enum and display it as 0°, 90°, etc.

Alternatively, maybe rotation can be defined as a floating point value? Is there value in accommodating arbitrarily rotated elements?

I'm fine with naming the property/values whatever the consensus says is the most clear. I chose cardinal directions to emphesise that "this is the way the texture is facing" rather than "this is how much I want to rotate the texture".

The value should be restricted to 90 degree turns and should not be a float accomidating arbitraty rotations. Other rotations do not benefit packing effeciency and are not lossless and should not be used in atlases.

(Also, can anyone help me out with the Style Check? I don't know how to fix it. I used --doctool, I used ClangFormat with visual Studio, this is my first pr to godot)

[RedMser]

@onesuchkeeper You did not bind the enum correctly it appears, since its documentation is also not being generated by doctool.

Looking at another class, this might be the missing piece?

godot/scene/3d/lightmap_gi.cpp

Line 1895 in 2a3e0d4

BIND_ENUM_CONSTANT(BAKE_QUALITY_LOW);

[Lielay9]

Using cardinal directions for the enum feels a bit weird. I understand the idea of using the face of a compass to indicate where the top of the element, but I think the following would be more clear:

• Name the enum rotation instead of Direction

• Have the values be 0, 90, 180, 270

• In code, name the elements something that's a valid identifier, like ROTATION_0_DEG, etc.

• If more user-friendly displayed values are desired, the editor can have a special case for this enum and display it as 0°, 90°, etc.

I'm sorry, but I strongly disagree. Using rotations in the enum is simply ambiguous. To understand the code, one has to read the documentation on which orientation 90 and 270 refer to; clockwise or anticlockwise. As far as I'm aware, compass is read clockwise, 4 cardinals corresponding to 90 degree steps, starting from north, everywhere. So not only does it imply where the top side of the rotated image lays, which mind you doesn't require rotating the image in ones head, it also implies an unequivocal rotation direction and definite set of possible values.

[groud]

I think the feature makes sense.

Typical sheets will only use 'North' and 'East' so the existing enum 'Orientation' could be used instead of 'Direction', but I feel handling all 4 cardinal will be better in the case an abnormal sprite sheet is made.

In general, we prefer to avoid future-proofing things. I thought a boolean would be good, but, while I don't see any reason to use the "south" orientation, I guess some packing tools might arbitrarily rotate sprites right or left.

I think I would consequently remove the "south" value, and keep the other two. That would avoid crowding the options with something I believe is useless.

Edit: besides that, I think the implementation looks good.

[Lielay9]

In general, we prefer to avoid future-proofing things. I thought a boolean would be good, but, while I don't see any reason to use the "south" orientation, I guess some packing tools might arbitrarily rotate sprites right or left.

I think I would consequently remove the "south" value, and keep the other two. That would avoid crowding the options with something I believe is useless.

While rotating 180/south (and mirroring for that matter) can't save space in general rectpacking algorithms, it is a possible way to eliminate empty space inside textured rectangles in algorithms made specifically for texturepacking. Example from my own experimental algorithm:

I do admit though that in general, packers capable of such are rare and it is more likely to find a atlases that have south facing images for no benefit at all, e.g. after merging multiple atlases:

So I wouldn't really call it "future-proofing" as much as I would call it "idiot-proofing".