Skip to content

Remove Irrlicht GUI gradients - #16015

Merged
sfan5 merged 1 commit into
luanti-org:masterfrom
siliconsniffer:no-gradients
May 7, 2025
Merged

Remove Irrlicht GUI gradients#16015
sfan5 merged 1 commit into
luanti-org:masterfrom
siliconsniffer:no-gradients

Conversation

@siliconsniffer

@siliconsniffer siliconsniffer commented Apr 12, 2025

Copy link
Copy Markdown
Contributor

This PR is related to #15826, see this discussion #6733 (comment).

Disclaimer: I have close to zero experience with irrlicht, this is a first hands-on experience for me.

To do/discuss

This PR is a Work in Progress. As far as I can tell the main concerns are:

  • missing contrast, could be mitigated with different brightness
  • harder to recognize separate buttons in groups of buttons
  • main menu tab color
  • harder to distinguish from text fields, needs to still look clickable
  • addition of colors
  • 9-slice

Screenshots

image
RE: removing the faux 3d stuff - let's keep that in.
image
#15826 (comment)
image

@SmallJoker

SmallJoker commented Apr 12, 2025

Copy link
Copy Markdown
Member

This PR does generally the same as #13718.

My opinion on this topic is unchanged since #13718 (comment). An API or the use of 9-sliced images might be needed to cover most use-cases and personal preferences.

@appgurueu

Copy link
Copy Markdown
Contributor

This PR does generally the same as #13718.

It looks like this PR addresses at least one major concern (contrast) which was brought up on the original PR by about four different people, and addressing more concerns is planned.

I prefer the current buttons. Others may prefer flat ones.

Most people seem to prefer flat ones (see e.g. the reactions on roller's PR).

A general solution would IMO be to use 9slice drawing for all buttons so that games and players could overwrite the textures (normal, hover, press) on demand.

The problem of button stylability is already pretty much solved as far as games are concerned. Try e.g. Mineclonia.

The question that remains is how default buttons, in the main menu, before the user has done any configuration, should look. The default should look appealing to most (first time) users, and currently it just doesn't.

People don't want to have to configure the GUI of an application to look okay. Some applications let you choose a theme, many don't. I don't think we should maintain multiple themes or a bunch of settings, or introduce something like a "mainmenu texture pack" or direct users to edit some files if they want the main menu to look okay.

It is on us to sensibly choose this default, like any other default, and this was, as far as I know, never done: We just inherited the Irrlicht default styles.

And I don't think the gradient is currently the best approximation of what people expect buttons to look like.

Comment thread irr/src/CGUISkin.cpp Outdated
@wsor4035 wsor4035 added @ Client / Audiovisuals UI/UX Formspec Roadmap The change matches an item on the current roadmap labels Apr 12, 2025
@Zughy

Zughy commented Apr 12, 2025

Copy link
Copy Markdown
Contributor

Concerning the 3rd screen: the red X might work but I wouldn't make the Login button green (definitely not that green, but anyway it can be argued that register and login bear the same weight - so same colour)

@siliconsniffer

siliconsniffer commented Apr 13, 2025

Copy link
Copy Markdown
Contributor Author

Concerning the 3rd screen: the red X might work but I wouldn't make the Login button green (definitely not that green, but anyway it can be argued that register and login bear the same weight - so same colour)

Thanks for your feedback. Keep in mind that this was just an "experiment", I wouldn't merge it like that either.
The green background color was a personal note, since the "default action" i.e. pressing enter triggers the login (if there is a username of course) - but that's by no means necessary and I completely agree that the color isn't fitting really well.
The red button might need some adjustment though, maybe change it to a regular instead of an image_button?
A more subtle red would also be beneficial.

Comment thread irr/include/IGUISkin.h
@siliconsniffer

Copy link
Copy Markdown
Contributor Author

Although I was previously more on the "let's find a compromise" side of things, I totally agree with @appgurueu.
It's, in my opinion, not feasible to find a compromise for the few people that prefer the gradients without creating an overly complicated solution for something that is meant to be provisional anyway.

In my opinion, we should move forward with this with only minor (e.g., color or contrast) changes before stalling again.
Maybe this could go into the RC? If enough people are upset by the change we can still roll it back.
In the end, this choice is upsetting people either way, but as appgurueu said, it's on us to choose the default.

@sfan5
sfan5 self-requested a review April 23, 2025 14:17
@sfan5

sfan5 commented May 4, 2025

Copy link
Copy Markdown
Member

So here's the devtest formspec:
Bildschirmfoto_2025-05-04_10-36-16
Bildschirmfoto_2025-05-04_10-36-36

It looks fine, but why did the tabs change color? -> EGDC_3D_FACE affects more than just buttons

re the open points:

addition of colors

out of scope.

9-slice

not sure what this means. we're editing the built-in button style, the option to use 9-slice for styling already exists.

Comment thread irr/src/CGUISkin.cpp Outdated
@sfan5

sfan5 commented May 4, 2025

Copy link
Copy Markdown
Member

minimally testable proposal with the other color:
Bildschirmfoto_2025-05-04_11-03-10
(the tabs change color too but it's more subtle)

diff --git a/irr/src/CGUISkin.cpp b/irr/src/CGUISkin.cpp
--- a/irr/src/CGUISkin.cpp
+++ b/irr/src/CGUISkin.cpp
@@ -24,7 +24,7 @@ CGUISkin::CGUISkin(EGUI_SKIN_TYPE type, video::IVideoDriver* driver)
        {
                Colors[EGDC_3D_DARK_SHADOW]     = video::SColor(101,50,50,50);
                Colors[EGDC_3D_SHADOW]          = video::SColor(101,130,130,130);
-               Colors[EGDC_3D_FACE]            = video::SColor(220,100,100,100);
+               Colors[EGDC_3D_FACE]            = video::SColor(185,85,85,85);
                Colors[EGDC_3D_HIGH_LIGHT]      = video::SColor(101,255,255,255);
                Colors[EGDC_3D_LIGHT]           = video::SColor(101,210,210,210);
                Colors[EGDC_ACTIVE_BORDER]      = video::SColor(101,16,14,115);
@@ -158,7 +158,7 @@ CGUISkin::CGUISkin(EGUI_SKIN_TYPE type, video::IVideoDriver* driver)
        for (u32 i=0; i<EGDF_COUNT; ++i)
                Fonts[i] = 0;
 
-       UseGradient = (Type == EGST_WINDOWS_METALLIC) || (Type == EGST_BURNING_SKIN) ;
+       UseGradient = false;
 }
 
 

@sfan5 sfan5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fundamentally we should do this.
The GUI style is still not pretty but it's no longer offensively ugly.

@siliconsniffer

Copy link
Copy Markdown
Contributor Author

It looks fine, but why did the tabs change color? -> EGDC_3D_FACE affects more than just buttons

I can't tell, not familiar with how irrlicht actually works under the hood, but that has been noted multiple times already.

re the open points:

9-slice

not sure what this means. we're editing the built-in button style, the option to use 9-slice for styling already exists.

If I understood him correctly, @SmallJoker brought it up because he found that buttons are harder to distinguish from text fields with this change (see #13718 (comment)). Maybe this could be moved into a separate PR together with a discussion about the addition of colors?

@Zughy also spoke in favor of leaving the colors untouched for now (#15826 (comment)). Crossed it off the list.

@appgurueu

Copy link
Copy Markdown
Contributor

can't tell, not familiar with how irrlicht actually works under the hood

See CGUISkin::drawColored3DTabButton

@siliconsniffer

siliconsniffer commented May 4, 2025

Copy link
Copy Markdown
Contributor Author

Will look into it, thanks for pointing it out.
What is the plan with this now? Put it in the RC?
Edit: Ah, just saw the ongoing meeting. Will ask there.

@Desour

Desour commented May 4, 2025

Copy link
Copy Markdown
Member

How can you get the round buttons back in a game?

(For my (unchanged) opinion on this, see #13718 (comment).)

@siliconsniffer

Copy link
Copy Markdown
Contributor Author

I don't think formspec styling supports gradients, I guess you would need to have a gradient texture and use image_buttons.

@sfan5

sfan5 commented May 4, 2025

Copy link
Copy Markdown
Member

How can you get the round buttons back in a game?

9-slice may be able to emulate it to some degree, but since it's texturing vs. perfectly smooth vertex colors it will be subpar.
Personally I don't consider this a problem.

@sfan5 sfan5 changed the title Adopt 13718: remove irrlicht gradients Remove Irrlicht GUI gradients May 4, 2025
@grorp

grorp commented May 5, 2025

Copy link
Copy Markdown
Member

Can confirm that the new color proposed by sfan5 should be the one in the middle of the old gradient. That makes sense.

original PR
screenshot 1

my new color proposal
screenshot 2

sfan5's newest color proposal
screenshot 3

The newest color is somewhat in the middle between the other two. Fine by me.

@grorp grorp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree that this is an improvement.

@appgurueu appgurueu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I still support this.

p.s. when squashing, make sure to preserve at least co-authorship attribution for rollerozxa.

- No gradient background, added dark background.
- Removed unnecessary debug names and enums.

Co-authored-by: rollerozxa <rollerozxa@voxelmanip.se>
Co-authored-by: grorp <grorp@posteo.de>
@nauta-turbidus

Copy link
Copy Markdown
Contributor

I don't think ripping out the gradient code is a good idea really. Disable it by default, sure, if you want it. I personally like the gradients, I don't understand the criticism, but I don't care that much either. Rip out? Why?

@sfan5

sfan5 commented May 6, 2025

Copy link
Copy Markdown
Member

What would be the advantage of keeping it? It's dead code either way.

@nauta-turbidus

Copy link
Copy Markdown
Contributor

What would be the advantage of keeping it? It's dead code either way.

Allow modders to enable it in formspecs on purpose...?

@MisterE123

Copy link
Copy Markdown
Contributor

I'm glad for this change

@sfan5

sfan5 commented May 7, 2025

Copy link
Copy Markdown
Member

What would be the advantage of keeping it? It's dead code either way.

Allow modders to enable it in formspecs on purpose...?

We decided that we didn't want this.

@sfan5
sfan5 merged commit 9b2ee1d into luanti-org:master May 7, 2025
@appgurueu

appgurueu commented May 7, 2025

Copy link
Copy Markdown
Contributor

We decided that we didn't want this.

I agree that it's not important, but I genuinely think using 9-slice for this should probably work okay (I see no good reason why it wouldn't, an image of a gradient with reasonable resolution should be scaled reasonably - the filter isn't even very important). It probably won't match the current gradients pixel-perfectly but it should very similar if done right.

I'm too lazy to recreate an ugly button style using 9-slice though.

@Montandalar

Copy link
Copy Markdown
Contributor

Beauty is in the eye of the beholder. But 9-slices for gradients it will be. Those don't take much memory/storage anyway. And you might want a different gradient algorithm/colour stops anyway.

mjz19910 pushed a commit to mjz19910/luanti that referenced this pull request May 8, 2025
Co-authored-by: rollerozxa <rollerozxa@voxelmanip.se>
Co-authored-by: grorp <grorp@posteo.de>
omicron1100 pushed a commit to omicron1100/luanti that referenced this pull request May 14, 2025
Co-authored-by: rollerozxa <rollerozxa@voxelmanip.se>
Co-authored-by: grorp <grorp@posteo.de>
@farooqkz

Copy link
Copy Markdown
Contributor

Hello. I recently did a git pull, re-compiled and saw the change. New buttons are much better. I appreciate it @siliconsniffer! Keep up the good work!

@siliconsniffer
siliconsniffer deleted the no-gradients branch May 16, 2025 18:21
SmallJoker added a commit to SmallJoker/minetest that referenced this pull request Jun 8, 2025
SmallJoker added a commit to SmallJoker/minetest that referenced this pull request Dec 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.