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

Reminder to remove metadata from textures before each release #1831

Closed
paramat opened this issue Jul 17, 2017 · 28 comments
Closed

Reminder to remove metadata from textures before each release #1831

paramat opened this issue Jul 17, 2017 · 28 comments

Comments

@paramat
Copy link
Contributor

paramat commented Jul 17, 2017

Often useless metadata gets included in textures so this should be removed.

However, it is important to make sure the compression only removes metadata and does nothing else.
Usually, a compression program will convert RGBA textures to 'indexed colour' mode, this breaks leaves textures ('allfaces_optional' drawtype) as the transparent pixels have important colour information that is used when rendered as opaque.

@TumeniNodes
Copy link
Contributor

From what I have seen, while looking into this on the gaming level... a file which is 16px/16px will always use the same amount of memory of a graphics card as any other 16px/16px file.

The area where reducing the file's size counts, in this case..., is related to the load on the cpu.
Is this correct? The information I am seeing dates back to 2013/14 and is a generalized to the gaming industry with no specific game in mind.

I plan to start going through all of the textures for all MTG related mods in a few days, one at a time and test each as I go.
I just want to be clear on the reasoning behind this so I understand what I am doing it for, just for my own learning/understanding.

I also see this is best done on a per file basis, rather than done in a batched process. Which makes sense.
So any info others may have would be appreciated. I want to help as best I can, which seems to be in areas which are trivial, but tend to take time away from devs who could be working on far more important issues.

@paramat
Copy link
Contributor Author

paramat commented Aug 17, 2017

The area where reducing the file's size counts, in this case..., is related to the load on the cpu.
Is this correct?

Seems so. I am no expert.

Most MTGame textures have been 'optimised' 1 or more times already, but newer ones may not be, and stripping all metadata is useful.

@TumeniNodes
Copy link
Contributor

I guess it is also relevant to keeping the overall size of the software/project down, each little bit keeps download size and time down.

Reading through information on the optipng guide, so many variables involved, and there is still some areas which are being studied because not everything related is even known ha.
http://optipng.sourceforge.net/pngtech/optipng.html

Anyway, I see it as very simple and straight forward in relation to the needs for MTG textures and I already have a basic process I use but, a lot of interesting reading there.

@TumeniNodes
Copy link
Contributor

What are people's feelings toward using .jpg for all textures which do not use transparency?
Could be fairly beneficial
But, would it bring too much complexity combining/mixing .png and .jpg?

@sfan5
Copy link
Member

sfan5 commented Sep 12, 2017

.jpg

just no

@TumeniNodes
Copy link
Contributor

heh, alrighty then.
Just sayin', when dealing with 16px there is absolutely no noticeable change (other than in file size)

@SmallJoker
Copy link
Member

@TumeniNodes JPEG is the worst idea for pixelart, where each pixel should be displayed how it was originally drawn. The smaller or unnatural an image is, the more unsuited it is for this compression method. JPEG takes 8 by 8 pixels, compares it with a value table to figure out the best matching frequencies (DCT) to represent them. So this procedure would happen only four times (!) for one texture file, which logically results in really bad outputs.

@TumeniNodes
Copy link
Contributor

@SmallJoker
Your argument is factual and logical... ok, scrap the .jpg suggestion then.
I should have looked further into this before I mentioned it.
/me goes back to optimizing .pngs

@paramat paramat removed this from the 0.5 milestone Apr 14, 2018
@paramat paramat closed this as completed Apr 14, 2018
@paramat paramat changed the title Reminder to compress textures with optipng just before release Reminder to strip metadata from textures with optipng just before release May 31, 2019
@paramat paramat added this to the 5.1.0 milestone May 31, 2019
@paramat
Copy link
Contributor Author

paramat commented May 31, 2019

Re-opening for 5.1.0.

@TumeniNodes
Copy link
Contributor

TumeniNodes commented Jul 27, 2019

Be sure to set files back to rgb before using optipng, otherwise the file will read as already being optimized.
This is what I suspect happened with these larger files, they were set to use indexed without being optimized first

Optipng will set the file to indexed automatically once the optimization is complete.
Set the file to rgb then optipng filename -o7

@paramat
Copy link
Contributor Author

paramat commented Jul 28, 2019

Thanks. Once we're approaching release, i hope you can be our texture optimisation person.

@TumeniNodes
Copy link
Contributor

sure, with the understanding I will be referred to as Optimus Prime during that time frame

@paramat
Copy link
Contributor Author

paramat commented Sep 28, 2019

@TumeniNodes freeze has started so a good time to do some optimisation if you feel like it.

Important: Textures of nodes with drawtype 'allfaces_optional' (leaves) are intentionally RGBA and need to stay that way.
See:
https://github.com/minetest/minetest/blob/b79741c90ffffac6fb24783b38c5b507316cbcc8/doc/texture_packs.txt#L181
https://github.com/minetest/minetest/blob/b79741c90ffffac6fb24783b38c5b507316cbcc8/doc/texture_packs.txt#L191

@TumeniNodes
Copy link
Contributor

Yep, already started going through. And I am aware of that drawtype.

@paramat
Copy link
Contributor Author

paramat commented Oct 3, 2019

IRC:

tumeninodes> Hi paramat, A couple items I cannot recall if we need to leave rgb (grass and snow sides-like textures, signs textures)

paramat> ugh, actually i think we have left this too late, freeze is for looking for bugs, so texture changes should have been done before freeze, not your fault, ours. also, such a big change will be hell to review in 1 or 2 days. so, no rush, this may have to wait until after release

Additionally, we need time to check which textures need to be RGBA instead of indexed.
Removing from milestone, wasn't critical anyway.

@paramat paramat removed this from the 5.1.0 milestone Oct 3, 2019
@paramat paramat changed the title Reminder to strip metadata from textures with optipng just before release Reminder to strip metadata from textures with optipng just before release (sorry, 'just before' wasn't a good idea) Oct 3, 2019
@TumeniNodes
Copy link
Contributor

Once we have a list of those which need to RGB, might be a good idea to document somewhere (as a list and not hidden within the API for each drawtype) for any future texture creators or changes being done to existing ones?

@paramat
Copy link
Contributor Author

paramat commented Oct 4, 2019

Yes, if any else require a certain form it should be added in this section https://github.com/minetest/minetest/blob/81c2370c8b1a66a279a5ff450c78caf5dfef77bf/doc/texture_packs.txt#L181
Because no others are documented there i suspect no others need to be RGBA.

'Grass side' type textures are fine as indexed, they all appear ok in-game so if some are indexed all can be.
Signs textures seem ok indexed, again, they appear ok in-game so if some are indexed all can be.

Perhaps you can list here any non-leaves textures that you discover that are RGBA, for consideration?

@TumeniNodes
Copy link
Contributor

Perhaps you can list here any non-leaves textures that you discover that are RGBA, for consideration?

Any I come across as I go and with testing, I will certainly list.
PR will be ready to go right after the release

@paramat paramat closed this as completed Oct 20, 2019
@paramat
Copy link
Contributor Author

paramat commented Aug 27, 2020

Reopen as reminder for every release.

@paramat paramat reopened this Aug 27, 2020
@paramat paramat changed the title Reminder to strip metadata from textures with optipng just before release (sorry, 'just before' wasn't a good idea) Reminder to strip metadata from textures with optipng before each release Aug 28, 2020
@paramat paramat added this to the 5.4.0 milestone Aug 28, 2020
@TumeniNodes
Copy link
Contributor

I had done most but got completely tied up with real world stuff and Leukemia crap.
Not even sure where I left off or where I put the file now (because I work in a chaotic method) :P

But, better than setting a reminder to strip metadata prior to each release, make it a mandate for any new texture approvals (problem solved).

See how the current PR for this goes. If it goes ok = good, if not I'll just start from scratch and would only take a couple days
Though that current PR seems it was done as a batch job, which honestly isn't the best method.

Go through each individually the one time, and then make it a requirement (just as code has certain requirements for approval)

@paramat
Copy link
Contributor Author

paramat commented Aug 30, 2020

No problem, there is no rush to do this, 5.4.0 is distant. I will bump this issue when we are close to release.
I agree that textures should be right first time, as these endlessly repeated compression PRs are annoying =)
The current open PR is useless at the moment, and should be done close to release to avoid duplicated work.

@An0n3m0us
Copy link
Contributor

I haven't followed the conversation here, so I'm wondering why not use a simple command to strip the metadata from multiple files and then make the requirement?
https://unix.stackexchange.com/a/490925

@paramat paramat changed the title Reminder to strip metadata from textures with optipng before each release Reminder to remove metadata from textures before each release Aug 31, 2020
@TumeniNodes
Copy link
Contributor

Sometimes batch processes remove more or less than what you intend.
Then you have to go through and check them all, which sort of defeats the idea of a "batch process".

Personally, I prefer to go through each, individually, set them back to rgb, remove the metadata, then optimize.
This way you know it's done once and for all, and everything is the way it should be, nice n clean.
Sometimes "hand done" is just better. And it really doesn't take that long.

@TumeniNodes
Copy link
Contributor

TumeniNodes commented Sep 5, 2020

I forgot to mention, that all the images should be using sRGB as the base profile before indexing, and I have come across some which are RGB, which can also lead to higher file size after indexing.
Not by a significant amount, but still higher

@paramat
Copy link
Contributor Author

paramat commented Sep 5, 2020

I suggest we investigate a compression program carefully, perhaps optipng, and discover the configuration that strips metadata and does nothing else. Then compressing in batch will be ok.
Knowing the correct configuration will be very useful for other projects and batch-processing will save a lot of time.

@TumeniNodes
Copy link
Contributor

TumeniNodes commented Sep 5, 2020

Compression/optimizing = indexing involved, which as mentioned numerous times now, does not work for allfaces drawtype.
I'm sure someone with the time to create a script for a batch editor could exclude those from the process, or prob already a means to do so (I don't use batch processing since a long time now)
For me, personally, batch processes take the "hand crafted"-ness out of everything : /

But this is just the difference between programmers and artists.
If a programmer could create code to sleep for them, they'd do it xD
two different forms and views of "art work"

optipng for me is the most reliable and easy to use.
And this honestly should be a one time deal, and will be, once a guideline is set for submitting textures.

Either way, I'm picking a day next week to finish up the folder I had set aside, and was already 1/2 through.

@paramat
Copy link
Contributor Author

paramat commented Sep 7, 2020

Ok cool, go for it.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Feb 3, 2021

Obligatory 5.4.0 bump.

@sfan5 sfan5 removed this from the 5.4.0 milestone Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants