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

Add methods to get target filter and repeat #66546

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Sep 28, 2022

get_texture_filter() and get_texture_repeat() in CanvasItem return the filter/repeat of the current node, but if it's set to "inherit", the returned value isn't what one would expect. To get accurate value, you'd need something like is_visible_in_tree(), but for filter and repeat.

Luckily such thing exists and it's called texture_filter_cache and texture_repeat_cache. This PR exposes these properties via public methods get_target_texture_filter() and get_target_texture_repeat().

This is needed for #60149 and #57596
The methods are not exposed to scripting for now.

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 good to me!

@groud
Copy link
Member

groud commented Oct 19, 2022

Looks good to me, my only concern is the naming which it kind of weird, as I am not really sure what "target" means here.
I think a more adequate naming should probably copy the one for CanvasItems' visibility, which uses is_visible_in_tree(). Thus I think it could be renamed to get_texture_filter_in_tree() and get_texture_repeat_in_tree()

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 19, 2022

Renamed.

@clayjohn clayjohn merged commit a8c805b into godotengine:master Oct 19, 2022
@clayjohn
Copy link
Member

Looks great! Thanks

@reduz
Copy link
Member

reduz commented Oct 19, 2022

I think this PR is good and it makes sense it was merged, but given this is core engine I would appreciate being asked next time.

@reduz
Copy link
Member

reduz commented Oct 19, 2022

Anything SceneTree or core nodes (Node, CanvasItem, Control, Node2D, Node3D, Viewport, etc) is core.

@KoBeWi KoBeWi deleted the rfeipletaetr branch October 19, 2022 21:10
@MrBlockers
Copy link
Contributor

I'm making use of this for #67288 but discovered that it doesn't seem to respect the default texture filter project setting when the all of the nodes up the tree are set to inherit. It seems like it's giving me linear in a project where all the nodes are set to inherit but the project setting is nearest. Should I open an issue for that?

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 23, 2022

Yeah that sounds incorrect.

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.

5 participants