-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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 ability to change Icon Saturation #45924
Conversation
-Allows for more theme freedom -Allows for entirely B&W themes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I am not sure about how useful that is, but it kind of make sense to be able to customize the look of icons.
My only concern might be about this new adjust_bcs
function. I wonder if it not too specific to our use case for it to be exposed. Though it's likely a basic filter so why not.
I believe we really have to discuss what we want the Image class to expose, that's the kind of class which could end up with hundred of methods if we don't set a border between what we accept and what we don't.
@groud image class has lots of filters already. Users use those a lot (in fact most came from PRs). Additionally, this new one is tiny, so no harm done. |
Well, no. I can't see any filter in the Image class. We have some image manipulation functions (crop, copying rects, etc...) but I can't find any filtering function. |
@groud I guess you are right, it's more manipulation than filters. Well, in any case, I am not against adding more stuff to Image as needed anyway. I will just not do it preemptively (or with previous discussion) though, if someone needs a filter it gets added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see some more image processing methods being added to core. adjust_bcs
is general-purpose enough, and would be useful for open-source projects like https://github.com/Orama-Interactive/Pixelorama.
@Xrayez I am open to doing more work on this stuff if users need it, but they need to ask for something they need first. In this case I needed it myself, but adding more image processing is welcome. |
Thing is that the amount of requirements for users might be infinite. If we start to consider image manipulation tools as a valid and common enough use case for Godot, there's no reason not to implement a lot of other features in the Image class. I am ok with adding basic filter for now (like color adjustment, de-saturate, etc...) But, IMHO, we should really draw a strict limit somewhere beforehand. |
@groud I agree, but it depends what they ask for. Let me explain the context better so you have a better idea. With the improvements in 4.0, it is now much less costly to do image manipulation or other operations in Image, and the same goes submitting the changes of an Image (via Texture) to RenderingServer as well. In 3.x this has a lot of penalties, like having to lock, or the image having to be copied when submitted (which is costly). These improvements now allow users, (who may want to do fancy pixel effects for stuff, or even generate textures procedurally) to do it directly on CPU on an Image and see the changes in real-time at little cost. Before, a lot of this had to be done with shaders, or could not be done at all. An example of this is using a brush and painting on a texture (something very requested). Many game mechanics rely on this, but this was expensive in 3.x, and very cheap in 4.x. Users will see that a lot of possibilities open with this and will most likely ask for more features, so we should be open to implement what is reasonable. |
My 2 cents/opinion. Games are very art dependent creations so any features that allow for more visual art control in the game are beneficial in my opinion. It opens the gates for new creative ideas. |
Examples: