Auto resize icon which passed by add_custom_type().

[Daylily-Zeleen]

Before:

After:

---

Need #71872 .

[akien-mga]

Note for reviewers, to be checked together with #71817 (could have been kept in the same PR IMO, but now that both PRs are open it's fine to keep it like this).

[Daylily-Zeleen]

Note for reviewers, to be checked together with #71817 (could have been kept in the same PR IMO, but now that both PRs are open it's fine to keep it like this).

I am not sure about these two pr should be merge as one or not, icon size issues is scattered, some are fixed and some are remained, so I keep them separated, too.

[YuriSizov]

I'm not sure about that approach for either of the PRs. Why waste resources resizing the loaded resource when we can render the texture at any scale as we need it? Especially in the inspector, which is a custom control.

[Daylily-Zeleen]

I'm not sure about that approach for either of the PRs. Why waste resources resizing the loaded resource when we can render the texture at any scale as we need it? Especially in the inspector, which is a custom control.

I think this implemention just to keep style consistently.
This pr is different to #71817 , the passed icon resource will not be refrenced, just regenerate a new resized texture and reference it, and the new texture only be used by built in functions.

In other word, generate once and can keep style consistently, if user want to create a custom inspector control, they can use the origin texture directly.

Additionally, there have some icon issues about icon size in inspector, such as #68962, I think user prefer use a any size of texture as icon to use a fixed size texture, it means that It's worth to implement auto resize icons. This is my reson of implement #71817 , although the implemention of it is not perfect.

---

Please check the origin implementition( before #71817 ):

1 ,2, 3 are use the same texture, but only 3( category icon) is not be auto resized. if we remaind it without auto resized, it is quite weird, right?

[YuriSizov]

@Daylily-Zeleen You are missing my point: why do we need to resize the icon ahead of time, spend time and resources on that and keep a copy, if we can just as well render this same original icon at 16x16, or 32x32, or whatever other size we need? draw_texture_rect can be given any size we want, directly in the drawing code, without any ahead of time resizing.

This is in fact how the icon is scaled in your image for the p.1 and p.2, with some internal logic of Button. Similar features are available in TextureRect, where you can clearly see different ways to do it.

[Daylily-Zeleen]

@YuriSizov I check the logic of p.2, the icon is implement by setting texture for a TextureRect instead of draw texture directly, so I think this pr to solve custom type icon which add by EditorPlugin.add_custom_type() is appropriate. ( I am not sure the logic of p.1, but it can be handled, too.)

However, the icon of p.3 is drawn directly, I will change the way to implement #71817 .

[YuriSizov]

I check the logic of p.1, the icon is implement by setting texture for a TextureRect instead of draw texture directly

That icon is part of the Button, but it doesn't matter. Both Button and TextureRect scale textures by passing the desired size to the draw_texture_rect method (or similar methods), which means that the scaling is done by the GPU and during render. The same should be done in every place that renders the icon passed to add_custom_type. We shouldn't be making copies of the icon and resizing it to an arbitrary value.

[Daylily-Zeleen]

Ideally I think you are right, but the p.2 implemention like this:

Button(EditorPath)
|_ MarginContainer
    |_ HBoxContainer
        |_ TextureRec( the custom type icon)
        |_ Label 
        |_ TextureRect( the arrow icon)

And use TextureRec::set_texture() to set icon.

If want to draw texture directly, I think it is more reasonable to provide ability of set size of button icon( for p.1) and allow set max size of Control or any other way to limit the size of TextureRect in Container( for p.2). obviously, this is another issue and need a propersal.
In other word, we should do larger change for the implemention of EditorPath(p.2), Button(p.1) and any other control which relevant to icon without draw it directly.

In this situation, origin implement of add_custom_type() will cache the the icon texture, I still think that just generate a new resized icon and discard the origin one is appropriate, use minimal change to avoid potential mistakes.

By the way, I change the implemention of #71817 as you wish, in that situation, draw the icon texture directly is more reasonable.

[YuriSizov]

Sorry, I got confused and thought that EditorPath already worked as expected. However, if it uses TextureRect, then all you need to do is set the correct scaling flag and it should shrink to the size of the container.

Wherever we use textures, we can scale them on the GPU. I don't think there is a case for resizing them ahead of time as an image, on the CPU. Either it's direct drawing and we have full control, or the node that we use implements it, or the node doesn't implement it, but it should (e.g. we have an issue with the popup menu, and it should be fixed by exposing a way to control it for the popup menu). Pre-resizing shouldn't be the answer.

[Daylily-Zeleen]

Sorry, I got confused and thought that EditorPath already worked as expected. However, if it uses TextureRect, then all you need to do is set the correct scaling flag and it should shrink to the size of the container.

Wherever we use textures, we can scale them on the GPU. I don't think there is a case for resizing them ahead of time as an image, on the CPU. Either it's direct drawing and we have full control, or the node that we use implements it, or the node doesn't implement it, but it should (e.g. we have an issue with the popup menu, and it should be fixed by exposing a way to control it for the popup menu). Pre-resizing shouldn't be the answer.

Okey, I avoid to pre-resize the icon texture now.
I sent a new pr to implement limit button icon to handle EditorResourcePicker.
For EditorPath, I change TextureRect's expend_mode and stretch_mode, then set custom_minimum_size to handle it's icon.

But I can't sure this two enhances can cover all situation of custom icon.

[YuriSizov]

Thanks for your contribution! I've incorporated all your work in #75472, so this is superseded now. You are properly credited in the relevant commit.