StyleBoxTexture stretching default behaviour is different in Godot 3.2.4rc (GLES3)

[sergey-khrykov]

Godot version:
3.2 (64a9e86)

OS/device including version:
macOS 10.15.7

Issue description:
StyleBoxTexture stretching in 3.2 (latest) is somewhat different than it was in 3.2. This will probably break the look of a number of apps switching to a future 3.2.4 release. Not sure if this is a bug or just new behaviour (what’s the point?)

Steps to reproduce:
Create an item (e.g., a button) with a StyleBoxTexture. In this example, we use a circle picture as a texture. For a wide range of height-to-width ratios, the roundings are considerably different. Run the project with the latest 3.2 and compare the look of the button in 3.2.3_stable.

Minimal reproduction project:
New Game Project.zip

[Calinou]

I can reproduce this on 3.2.4rc3, both with batching enabled and disabled in the Project Settings. Switching to the GLES2 renderer doesn't fix the regression either.

Also, when downsizing the button in a way that makes its corner radii larger than one of its sides, this happens:

[kuruk-mm]

I bisect it, and this was introduced in c2290db from @lawnjelly before 3.2.4beta1

He added this new setting:

<member name="rendering/quality/2d/ninepatch_mode" type="int" setter="" getter="" default="0">
    Choose between default mode where corner scalings are preserved matching the artwork, and scaling mode.
    Not available in GLES3 when [member rendering/batching/options/use_batching] is off.
</member>

If you put it as Scaling you would get the same result as 3.2.3-stable.

So I suggest to put the default value to Scaling, to work as 3.2.3-stable. And maybe rename the other value because the name is Default, so it's a bit confused.

I'm will make a PR changing the default value.

[lawnjelly]

Yes this is intentional, it isn't a bug.

If you look through the PR / linked issues there were two schools of thought with ninepatch behaviour, and only one was supported at different point releases of Godot (the old behaviour, or the puthre behaviour). Whichever was set, the other side would complain.

Now both methods are available with this switch. And the majority seemed to be in favour of the old method as default so we have switched back to that, we had to make a decision at the time (this was 6 months ago, it may have taken place in GLES2 first though).

Although you may be confused, if the default was set the other way, the other users would be confused. So there is no perfect solution. And it should be easy enough to fix, just flip the project setting rendering/2d/options/ninepatch_mode. 👍

Actually I probably should make that setting restart the editor so you can see it straight away.

Regarding the default, I have no particular allegiance to either, if we have enough votes / opinion for the other as default that's no problem, I think we just went for the most sensible decision we could do at the time (there was also a proposal about it).

godotengine/godot-proposals#1618

[sergey-khrykov]

Yes this is intentional, it isn't a bug.

If you look through the PR / linked issues there were two schools of thought with ninepatch behaviour, and only one was supported at different point releases of Godot (the old behaviour, or the puthre behaviour). Whichever was set, the other side would complain.

Now both methods are available with this switch. And the majority seemed to be in favour of the old method as default so we have switched back to that, we had to make a decision at the time (this was 6 months ago, it may have taken place in GLES2 first though).

Although you may be confused, if the default was set the other way, the other users would be confused. So there is no perfect solution. And it should be easy enough to fix, just flip the project setting rendering/2d/options/ninepatch_mode. 👍

Actually I probably should make that setting restart the editor so you can see it straight away.

Regarding the default, I have no particular allegiance to either, if we have enough votes / opinion for the other as default that's no problem, I think we just went for the most sensible decision we could do at the time (there was also a proposal about it).

godotengine/godot-proposals#1618

Thank you for your prompt reply. What is still confusing to me is that in Editor, you see no effect of that setting, but you do see it when you start a scene. I believe that somehow the picture Editor should also reflect one’s selected setting.

Also, I still cannot bring back the default behaviour in my real scene, with this setting in either option. So please don’t close this bug and give me some time to investigate this.

[lawnjelly]

Thank you for your prompt reply. What is still confusing to me is that in Editor, you see no effect of that setting, but you do see it when you start a scene. I believe that somehow the picture Editor should also reflect one’s selected setting.

Yes, my bad, it's a parameter on the project setting, it specifies whether a reboot is needed. I referred to it in my earlier post, will fix it probably tomorrow as part of another project settings PR.

Also, I still cannot bring back the default behaviour in my real scene, with this setting in either option. So please don’t close this bug and give me some time to investigate this.

See here for more info in case this affects you:
#42899 (comment)

[kuruk-mm]

So, in my opinion the 3.2 version must stay as compatible as possivel between their minor releases.

Maybe this kind of changes, would improve the engine a lot, but for end-user maybe can be and feels like 'my game broke in this new Godot version'.

So I would let the same behavior, and add new things instead of changing what is already working.

If someone want to broke something, we can do it in 4.0 haha.

Let me know what you think.

[sergey-khrykov]

To me mind, 3.2 should introduce only improvements and bug fixes and never change a known behavior of anything. Some new features are also Ok, if they don't break things.

About 4.0, I basically stand for the same. Quick adoption is a very good thing, indeed. But if breaking something is absolutely necessary and there is a matching functionality in the new world, I believe I'll manage to go with it :)

[akien-mga]

Since the "scaling" behavior was the only option in 3.2 through 3.2.3, I agree that it should be preserved as the default behavior. We should aim to minimize breakage for users upgrading to a new patch release (3.2.3 -> 3.2.4).

The "old" behavior is indeed preferred by a number of users, which would have had to either stay on a previous Godot release to keep it, or implement their own system. So changing the default behavior in 3.2.4 for their sake is not really useful as they're not going to upgrade to 3.2.4 unless we make it known to them that this actually changed.

So I'd suggest changing the default value indeed, and finding a better name for "Default" as @kuruk-mm mentioned. It's still easy enough for users who want the old behavior (and considered that 3.2 was a regression) to flip the switch and get the desired behavior reintroduced.

We can reconsider for 4.0 which behavior should be the default, I don't have a strong opinion either way, but yes, within the 3.2.x branch, we should aim to preserve compatibility.

[lawnjelly]

We can certainly flip the default and change the name for the default option.
The more tricky bit is here:

Also, I still cannot bring back the default behaviour in my real scene, with this setting in either option. So please don’t close this bug and give me some time to investigate this.

We need a minimum reproduction project for this, (probably in a separate issue so as not to confuse the two). We need to assess what is causing this difference in order to decide how to fix it.

This just shows how important it is to test betas early on - help us to help you! 🙂

[akien-mga]

Also, I still cannot bring back the default behaviour in my real scene, with this setting in either option. So please don’t close this bug and give me some time to investigate this.

Can you confirm that it still doesn't work after restarting the project? (i.e. set stretching mode to "Scaling", and restart)

[lawnjelly]

See here for more info in case this affects you:
#42899 (comment)

I'm just having a relook at this case above, which is the only case where the option isn't present. We weren't keen on adding both methods to the GLES3 shader at the time, but it may be possible using a hack @clayjohn came up with where it doesn't waste a conditional. It's a bit uncharted territory though so I'm a little unsure whether we should try and get it in before 3.2.4.

Whether this is the cause of your further problems, I don't know though, hence min reproduction project etc.

[sergey-khrykov]

Also, I still cannot bring back the default behaviour in my real scene, with this setting in either option. So please don’t close this bug and give me some time to investigate this.

Can you confirm that it still doesn't work after restarting the project? (i.e. set stretching mode to "Scaling", and restart)

Everything works fine, changing the setting results in the old behaviour.

[akien-mga]

Great, closing as fixed by #46856 then.