minetest / minetest_game

Minetest Game - A lightweight and well-maintained base for modding [https://github.com/minetest/minetest/]
http://minetest.net/
Other
1.42k stars 577 forks source link

Reminder to remove metadata from textures before each release #1831

Closed paramat closed 3 years ago

paramat commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

.jpg

just no

TumeniNodes commented 7 years ago

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

SmallJoker commented 7 years ago

@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 commented 7 years ago

@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 commented 5 years ago

Re-opening for 5.1.0.

TumeniNodes commented 5 years ago

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 commented 5 years ago

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

TumeniNodes commented 5 years ago

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

paramat commented 5 years ago

@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 commented 5 years ago

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

paramat commented 5 years ago

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.

TumeniNodes commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

Reopen as reminder for every release.

TumeniNodes commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

TumeniNodes commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Ok cool, go for it.

Wuzzy2 commented 3 years ago

Obligatory 5.4.0 bump.