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] Add Anisotropic Filter option for TextureArrays #51402

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

TokisanGames
Copy link
Contributor

@TokisanGames TokisanGames commented Aug 8, 2021

  • Adds an option to enable anisotropic filtering for TextureArrays at import or runtime.
  • Fixes typo in Texture::_bind_methods.
  • Sets TextureLayered default flags to the same as Texture defaults: Repeat, Filter, Mipmaps. (This also changes it for TextureArray [desired for HTerrain] and Texture3D, I'm not sure if that's ok)

Addresses point three in #50900.

I'm using TextureArrays in @Zylann 's hterrain. Here it's working as expected. The new FLAGS_DEFAULT on top and that plus anisotropic filtering on bottom. This is with an 8192x6144 (4x3x2k) texture array.

image

@clayjohn
Copy link
Member

clayjohn commented Aug 8, 2021

Sets TextureLayered default flags to the same as Texture defaults: Repeat, Filter, Mipmaps. (This also changes it for TextureArray [desired for HTerrain] and Texture3D, I'm not sure if that's ok)

We should look into setting a different default for Texture3D and for ArrayTexture. IMO the old default made more sense for Texture3D, but the new default makes more sense for ArrayTextures.

@TokisanGames
Copy link
Contributor Author

TokisanGames commented Aug 10, 2021

I have created a second commit, not yet squashed, that separates the bound enums defaults, and the create function.

This bit of code produces the following output.

func _ready():
	var t3d = Texture3D.new()
	print("T3d new flags: %d" % [ t3d.flags ])
	t3d.create(100, 100, 2, Image.FORMAT_RGB8)
	print("T3d object flags: %d, T3d default: %d" % [ t3d.flags, Texture3D.FLAGS_DEFAULT])

	var ta = TextureArray.new()
	print("TA new flags: %d" % [ ta.flags ])
	ta.create(100, 100, 2, Image.FORMAT_RGB8)
	print("TA object flags: %d, TA default: %d" % [ ta.flags, TextureArray.FLAGS_DEFAULT]) 

	get_tree().quit()
T3d new flags: 4
T3d object flags: 4, T3d default: 4
TA new flags: 7
TA object flags: 7, TA default: 7

Is this what you want?

@clayjohn
Copy link
Member

I'd prefer to not duplicate the Flags in both classes. Wouldn't it be enough for them to just use different default values? You could even consider adding a FLAGS_DEFAULT_TEXTURE_ARRAY and FLAGS_DEFAULT_TEXTURE_3D enums to TextureLayered.

@TokisanGames TokisanGames force-pushed the texture_array_anisotropic branch from bab8377 to e8465c0 Compare August 12, 2021 14:33
@TokisanGames TokisanGames force-pushed the texture_array_anisotropic branch from e8465c0 to fb609b2 Compare August 12, 2021 15:56
@TokisanGames
Copy link
Contributor Author

If I can have two enum defaults, that makes things much simpler so I don't have to work around the clunky auto-documentation and bindings systems, and since C++ doesn't allow overriding enums.

This squashed version now includes separate default enum entries.

func _ready():
	var t3d = Texture3D.new()
	print("T3d new flags: %d" % [ t3d.flags ])
	t3d.create(100, 100, 2, Image.FORMAT_RGB8)
	print("T3d object flags: %d, T3d default: %d" % [ t3d.flags, Texture3D.FLAGS_DEFAULT_TEXTURE_3D])

	var ta = TextureArray.new()
	print("TA new flags: %d" % [ ta.flags ])
	ta.create(100, 100, 2, Image.FORMAT_RGB8)
	print("TA object flags: %d, TA default: %d" % [ ta.flags, TextureArray.FLAGS_DEFAULT_TEXTURE_ARRAY])
T3d new flags: 4
T3d object flags: 4, T3d default: 4
TA new flags: 7
TA object flags: 7, TA default: 7

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great now! Thank you!

@akien-mga akien-mga merged commit 54d14a9 into godotengine:3.x Aug 13, 2021
@akien-mga
Copy link
Member

Thanks!

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