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

Remove TGA deprecation and put it in a ”do not deprecate again” list #13794

Closed
erlehmann opened this issue Sep 11, 2023 · 19 comments · Fixed by #13877
Closed

Remove TGA deprecation and put it in a ”do not deprecate again” list #13794

erlehmann opened this issue Sep 11, 2023 · 19 comments · Fixed by #13877
Labels
Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature. @ Script API

Comments

@erlehmann
Copy link
Contributor

erlehmann commented Sep 11, 2023

TGA support was deprecated sneakily in a docs PR.

TGA is a simple image file format that is easy to write and modify.

TGA is mainly used for generated textures in Minetest mods.

Once is coincidence, twice is sabotage!

TGA support was once removed in the past without any discussion beforehand.

Comments on removing the TGA support were ignored for some time.

TGA was added after complaints that this broke MineClone 2.

Removing TGA support breaks these mods:

  • mcl_maps (part of MineClone 2 and Mineclonia)
  • painting (only TGA export functionality I think?)
  • sitelen_pona_signs (signs mod for toki pona)
  • uc_signs (signs mod using unicode_text)
  • unicode_text
  • xmaps (like mcl_maps, but standalone)
  • tga_encoder (well … 😅)
  • maybe more? (I only listed the ones I know)

TGA is a simple and very useful texture format

  • Writing it consists literally of a short header and dumping bytes.
  • TGA is well-supported by image editors like GIMP or mtPaint.
  • Minetest TGA support is provided by under 220 lines of code.
    • I volunteer to provide enbypower to maintain that code.
    • The code is very unlikely to change much in the future.

“Just use PNG” sounds nice, but is not helpful

  • Existing images are stored as TGA server-side for these mods. Changing the encoder does not change existing files.
    • kay27 tried to convert stuff to PNG using external tooling and largely failed at that.
    • Converting these files to PNG would require someone to re-implement Minetests existing C code in Lua.
  • In-game handheld maps are one of the hardest items to create, players have to modify one whole chunk for that.
    • Thus, breaking those maps is generally considered what is known as a ”dick move”.
  • Minetest's PNG encoding is really bad. The checkerboard PNG from devtest is 20 times the size it should be.
  • Minetest only encodes 32 bit RGBA PNGs. It does not support colormaps, grayscale, A1R5G5B5 etc.
    • RGBA can never duplicate the colormap hack where you have two entries with the same color.
  • TGA has lower overhead, it is literally just a header and a payload. PNG has framing overhead.
    • This complicates PNG reader code and debugging generated textures.
  • Generating TGA files using the tga_encoder mod is much more backwards-compatible.

While these issues all seem solvable, for some reason no one has stepped forward to solve them in years. To me, Minetests PNG writer capabilities look more like a proof-of-concept than something that is supposed to support the generation of dynamic textures well.

I personally assume that people suggesting solutions involving re-encoding stuff as PNG neither make mods (besides devtest) that generate many dynamic textures, nor have they read much of the source code of the mods mentioned. I also assume that they have zero interest in actually putting in the work of fixing TGA-related breakage in mods. Please notify if you both advocate removing TGA support and commit to fixing all issues that will result from this (which includes at least the issues mentioned above).

Edit: As of 2023-09-15 no one who advocates removing TGA has promised to fix all the breakage it would cause.

Texture generation must be fast, results must be small

  • For very small textures, even an uncompressed TGA file is smaller than optimized PNGs.
  • Optimizing encoding of PNG files takes too long (seconds), even for relatively small files.
  • I found that just compressing a TGA file using zlib can easily beat optipng for Minetest use cases.
    • Thus, removing TGA support would prevent a huge texture size optimization for generated textures.

TL;DR

That I even have to make an issue for this stuff to get it undeprecated is a fucking waste of time.

@erlehmann erlehmann added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Sep 11, 2023
@erlehmann
Copy link
Contributor Author

If you are interested in texture optimization possibilities I can provide benchmarks. I have not included them because:

  • Benchmarks distract a lot from more important points.
  • I have talked about them a lot in the past – few people listened.
  • It is very easy to prove or disprove my assertions in only a few minutes.
  • Those who actually read the code of the mentioned mods would already know them anyway.
  • People often claim to be interested in benchmarks, but tend to only like benchmarks when those are in their favor.

@sfan5
Copy link
Member

sfan5 commented Sep 12, 2023

TGA support was deprecated sneakily in a docs PR.
[...]
TGA was added after complaints that this broke MineClone 2.

(this section does not contain any arguments)

Removing TGA support breaks these mods:

That is not necessarily an argument, if we deprecate something the authors obviously have to work to migrate to something else.
If this amount was huge then sure, but it doesn't look that way.

TGA is a simple and very useful texture format
Writing it consists literally of a short header and dumping bytes.

Missing explanation on why this is a desirable property.

TGA is well-supported by image editors like GIMP or mtPaint.

Not an argument.

Minetest TGA support is provided by under 220 lines of code.

This is an argument. I would weigh this against how useful TGA is and IMO it makes more sense to remove this niche format then keep it.
(To be clear this is not something urgent but maybe in a few years.)

Existing images are stored as TGA server-side for these mods. Changing the encoder does not change existing files.

You're talking about mods that generate images, but correct.

kay27 tried to convert stuff to PNG using external tooling and largely failed at that.

Converting images is trivial, if you guys can't manage that then I don't know what to say.

Converting these files to PNG would require someone to re-implement Minetests existing C code in Lua.

Lie.

In-game handheld maps are one of the hardest items to create, players have to modify one whole chunk for that.
Thus, breaking those maps is generally considered what is known as a ”dick move”.

I don't know what this is but it's not an argument.

Minetest's PNG encoding is really bad. The checkerboard PNG from devtest is 20 times the size it should be.

Correct. Our PNG encoder could use some improvements.

Minetest only encodes 32 bit RGBA PNGs. It does not support colormaps, grayscale, A1R5G5B5 etc.

Thankfully 32bpp RGBA can represent all colors the others can.

TGA has lower overhead, it is literally just a header and a payload. PNG has framing overhead.

Correct, but also largely irrelevant.

This complicates PNG reader code and debugging generated textures.

Where does such reader code exist?
Also as a modder you don't "debug" textures, if encode_png spits out an invalid file you file a bug and that's our problem. (If anything this is an argument for including libpng proper into the server.)

Generating TGA files using the tga_encoder mod is much more backwards-compatible.

PNGs are just as compatible as long as you can manage to use an one-year-old version on the server.

For very small textures, even an uncompressed TGA file is smaller than optimized PNGs.

How do I "easily prove this in only a few minutes"?
I think it sounds plausible however, how big is the difference? Does this difference justify keeping around TGA?

Optimizing encoding of PNG files takes too long (seconds), even for relatively small files.

Lie. Inform yourself of how optipng works before making such claims.

I found that just compressing a TGA file using zlib can easily beat optipng for Minetest use cases.

We don't have this, irrelevant.

@celeron55
Copy link
Member

celeron55 commented Sep 12, 2023

It seems to me the biggest argument for TGA is the case where images are used as a read-write data format on the server side within MT and with interop towards miscellaneous tooling. In that case you want exact control of what goes into the file to ensure it's readable AND re-writeable by whatever tooling is chosen. It's unlikely Minetest will support such exact control for PNG.

Thoughts? It's not exactly a widely understood concern among game engines. Read-write images are a similarly niche thing for pixel art as trackers are for retro music. Pixel art is the recommended way of doing art on MT though, so it may apply.

@rubenwardy rubenwardy added Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature. and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Sep 12, 2023
@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 12, 2023

It seems to me the biggest argument for TGA is the case where images are used as a read-write data format on the server side within MT and with interop towards miscellaneous tooling. In that case you want exact control of what goes into the file to ensure it's readable AND re-writeable by whatever tooling is chosen. It's unlikely Minetest will support such exact control for PNG.

Thoughts? It's not exactly a widely understood concern among game engines. Read-write images are a similarly niche thing for pixel art as trackers are for retro music. Pixel art is the recommended way of doing art on MT though, so it may apply.

This is a thing relevant to mcl_maps and xmaps. To create progressively updated in-game images or maps that can be zoomed out later or combined, you need a format that is easy and fast to write and read and allows maximum control.

A reason I have not done this yet is because I do not want to flood the client with unnecessary textures that will not be used ever again. I fear that would happen eventually, if I automatically updated a hand-held map whenever the world changed.

@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 12, 2023

TGA support was deprecated sneakily in a docs PR.
[...]
TGA was added after complaints that this broke MineClone 2.

(this section does not contain any arguments)

I wanted people new to this discussion to know that you aimed to remove TGA again.

Last time there was no discussion beforehand because it was in the separate IrrlichtMT repository.

This time there was (AFAIK) no discussion before and you told @Zughy to add it to some documentation PR.

Removing TGA support breaks these mods:

That is not necessarily an argument, if we deprecate something the authors obviously have to work to migrate to something else. If this amount was huge then sure, but it doesn't look that way.

Migration is only possible if ”something else” exists.

The amount of work to do it correctly (i.e. not half-assed) is probably a bit larger than you imagine.

As maintainer/contributor of several mods that generate dynamic textures, I think I can estimate that a bit better,.

TGA is a simple and very useful texture format
Writing it consists literally of a short header and dumping bytes.

Missing explanation on why this is a desirable property.

I have improved tga_encoder multiple times.

In the meantime, no one has improved Minetest's PNG encoding AFAIK.

I personally think this is probably because you need to do quite a bit more work to do the PNG thing.

TGA is well-supported by image editors like GIMP or mtPaint.

Not an argument.

I think if an image format is well-supported by editing software plays some role.

Minetest TGA support is provided by under 220 lines of code.

This is an argument. I would weigh this against how useful TGA is and IMO it makes more sense to remove this niche format then keep it. (To be clear this is not something urgent but maybe in a few years.)

In which ways are these 220 lines (that very rarely need to change) some kind of maintenance burden at all?

I already said I volunteer to maintain them., given that I may indeed have some domain expertise.

kay27 tried to convert stuff to PNG using external tooling and largely failed at that.

Converting images is trivial, if you guys can't manage that then I don't know what to say.

Well, kay27 also thought this was trivial, but it broke all maps on his MineClone 5 server.

IMO your response strongly suggests that you never actually tried to do the same thing as kay27.

Converting these files to PNG would require someone to re-implement Minetests existing C code in Lua.

Lie.

How would you propose to deal with entities and items that contain a TGA bitmap in their metadata otherwise?

In-game handheld maps are one of the hardest items to create, players have to modify one whole chunk for that.
Thus, breaking those maps is generally considered what is known as a ”dick move”.

I don't know what this is but it's not an argument.

This means you can not just /give players a replacement item, like if an update took away some diamonds or so.

Minetest's PNG encoding is really bad. The checkerboard PNG from devtest is 20 times the size it should be.

Correct. Our PNG encoder could use some improvements.

So do you have a good reason why the PNG encoder was never improved, while tga_encoder was?

Minetest only encodes 32 bit RGBA PNGs. It does not support colormaps, grayscale, A1R5G5B5 etc.

Thankfully 32bpp RGBA can represent all colors the others can.

Lack of knowledge on your part about colormaps and color formats and their use cases is not an argument in your favor.

TGA has lower overhead, it is literally just a header and a payload. PNG has framing overhead.

Correct, but also largely irrelevant.

This is relevant for both ease of programmatic use and theoretical maximum compression.

This complicates PNG reader code and debugging generated textures.

Where does such reader code exist?

I have no idea what you mean. Lots of applications have code to read PNG.

Also as a modder you don't "debug" textures, if encode_png spits out an invalid file you file a bug and that's our problem. (If anything this is an argument for including libpng proper into the server.)

Meanwhile, TGA is simple enough to debug using a hex editor.

Generating TGA files using the tga_encoder mod is much more backwards-compatible.

PNGs are just as compatible as long as you can manage to use an one-year-old version on the server.

“X is just as compatible as Y if you exclude all the cases where X is not as compatible as Y.”

For very small textures, even an uncompressed TGA file is smaller than optimized PNGs.

How do I "easily prove this in only a few minutes"?

By downloading tga_encoder and … looking at the source code of examples.lua?

I remember that last time i proved such a thing to you the issue in which I did was locked, by you.

I think it sounds plausible however, how big is the difference? Does this difference justify keeping around TGA?

I think that difference is a red herring. Almost everyone who has asked me about such things turned out to not care much about filesizes at all. See #13795 for a proposal on filesizes that you and others seemed to reject, even though it is trivial to implement.

Optimizing encoding of PNG files takes too long (seconds), even for relatively small files.

Lie. Inform yourself of how optipng works before making such claims.

The following command takes 2 seconds on my computer to spit out an optimized PNG:

optipng -o7 -zm1-9 games/devtest/mods/testnodes/textures/testnodes_generated_ck.png

You might have a faster computer.

I found that just compressing a TGA file using zlib can easily beat optipng for Minetest use cases.

We don't have this, irrelevant.

Again, see #13795 for my proposal.

Edit: I just realized that even without #13795 the zlib thing is relevant because that is currently the most efficient way to store a generated bitmap in a format that Minetest can render in item/entity meta. I obviously have to decompress it before doing anything with it, but xmaps and at least one version of mcl_maps do that. IIRC making item meta larger is an easy way to get more lag.

@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 13, 2023

It's not exactly a widely understood concern among game engines.

While games indeed do not often let users edit textures at runtime, many game engines actually have no issues with that scenario, e.g. loading player skins or other assets dynamically. Some of them also have suspicious functions that can write TGA files at runtime – and while I do remember an era in which this meant writing screenshots to disk, I'm pretty sure it is mainly used for other stuff in newer engines, since PNG is much better for screenshots that are never displayed by the engine (smaller filesizes & works on the web).

A very common scenario in game engines that do not use TGA as a texture format is actually that they use it as a source format in their asset pipeline for baiscally the same reasons you listed – wide interoperability and full control over every detail. For example, Valve's VTF format uses TGA it as an input format. You can actually buy assets online that are only delivered in TGA format – simply because that is what is easiest to edit and adapt.

@v-rob
Copy link
Member

v-rob commented Sep 13, 2023

It seems to me that there's a large multiplication of arguments to get TGA undeprecated, some of which don't even work well with each other. For instance:

TGA is well-supported by image editors like GIMP or mtPaint.

I found that just compressing a TGA file using zlib can easily beat optipng for Minetest use cases.

Last time I checked, I can't import or export a zlib-compressed TGA file using GIMP. I'd have to use multiple tools in conjunction with each other, not something very nice if I have to work with your mod. Multiplying arguments that don't work well with each other just makes it sound like you're grasping at straws here. For me to take these arguments seriously, I want them boiled down to the important arguments rather than a bunch of miniature argument points.

Applying the aforementioned simplification, these appear to be the main points in favor of TGA:

  • Maintaining backwards compatibility with existing (mod-generated) TGA images.
  • TGA is small and fast, saving on storage size, network transfer size, and generation speed, especially as compared to Minetest's current PNG encoder.
  • TGA makes working with certain types of modding situations and external tooling easier to do.

If we don't consider these points important enough to warrant keeping TGA in light of the (albeit small) maintenance burden, then no amount of further arguments will be convincing because they're all reiterations of the same stuff. I see more than one wall of text in GitHub and IRC that isn't bringing anything new to the table. Please don't waste our time; make your point and be done with it. If we decide against TGA while you still disagree, don't keep arguing endlessly. Understand that our decision is ultimately final, and continuing to argue will just make us ignore you.

@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 14, 2023

Last time I checked, I can't import or export a zlib-compressed TGA file using GIMP.

I think this is misrepresenting a point I made explicitly about transfer size and compression speed.

I probably need an external tool to load /usr/share/doc/optipng/optipng.man.pdf.gz in a PDF reader as well.

Multiplying arguments that don't work well with each other just makes it sound like you're grasping at straws here.

I am just listing everything I know. Some people actually claimed that TGA is not well-supported and that's not true.

Besides that, I do actually know someone who used mtpaint to make a 5×5 TGA texture set (not on ContentDB).

Applying the aforementioned simplification, these appear to be the main points in favor of TGA:

* Maintaining backwards compatibility with existing (mod-generated) TGA images.

* TGA is small and fast, saving on storage size, network transfer size, and generation speed, especially as compared to Minetest's current PNG encoder.

* TGA makes working with certain types of modding situations and external tooling easier to do.

I largely agree with, with three clarifications:

  1. IMO any possible filesize benefits are a red herring. Speed, features, and ease of use are much more important.

  2. Minetest's PNG encoding has never been improved since it was introduced, but tga_encoder is maintained.

  3. Being able to do stuff with textures in mods is much more important to me than than external tooling.

If we don't consider these points important enough to warrant keeping TGA in light of the (albeit small) maintenance burden, then no amount of further arguments will be convincing because they're all reiterations of the same stuff.

I agree.

I see more than one wall of text in GitHub and IRC that isn't bringing anything new to the table. Please don't waste our time; make your point and be done with it.

The reason I have been doing this is that in the past relentlessly repeating a point has worked every time when Minetest developers had little interest to educate themselves on an issue but held strong opinions regardless. I repeat stuff not so that people copy my opinions, but so that people actually pay attention to an issue and think about it enough to form an opinion that is not solely based on the first thing that comes to mind, folk wisdoms, or common misconceptions.

First and foremost, I want people to actually have an informed opinion. It's obviously a bonus if they agree with me.

My standard for “informed opinions” about file formats starts with ”read the specification” and also includes “understand when this is used and why people choose it” and ”understand the trade-offs in the available implementations”. I was told that it is pretty rude how I dismiss people's opinions if they haven't done their research, but what should I do other than loudly repeat myself in a case where the person not doing their research is in a position of power and has the advantage that they do not have to justify themselves? There was, after all, no ”deprecate TGA” ticket, just a sneaky docs change.

I know that repetition and pointing out when people say things that indicate they have no clue makes people dislike me (because it is severely annoying) and I try to only employ that tactic if I see people continuously dismiss things in a way that indicates they have not really put much thought to it. If they do put thought to it and disagree with me based on benchmarks or values or whatever, for me it is a good outcome by itself.

If we decide against TGA while you still disagree, don't keep arguing endlessly.

I'm pretty sure most things you will hear is complaints from users whose stuff breaks, i.e. MineClone 2.

A few times an engine change broke MineClone 2, I fixed those things for them. I will probably not do it again.

Understand that our decision is ultimately final, and continuing to argue will just make us ignore you.

I know that, and if that rug is pulled, I probably have no interest in making any more Minetest mods, as it would mean that obviously not even quite useful stuff can be relied on to be continued – and that not even one-of-a-kind mods like mcl_maps or unicode_text are safe from being broken. That has little to do with technical merits and much with the values a developer community wants to have – and if stuff is deprecated without even having suitable replacements, obviously getting rid of a little bit of code is more important than technical or social (i.e. stakeholder) considerations.

Besides that, just so you know: A TGA deprecation is something that affects mostly mods that I have improved, maintained or authored, so any fallout would probably affect me (e.g. users asking me to do a lot of work) … and AFAIK it was put into the docs without a ticket or even a small debate and was proposed by someone who, as far as I can tell, does not like me, does not maintain mods that create dynamic textures, keeps advertising a sub-standard replacement, demonstrated a lack of domain knowledge repeatedly and closed an issue after I explained how a proposed solution by them was not as good of an idea as they thought (I concede that I did it in a quite sarcastic way). To me this looks suspiciously not only like a technical discussion, but also some kind of power play, laser-focused on sabotaging software I am responsible for. I did not assume malice for quite some time, but enough people have told me that they think this might also be personal thing that I want to put it in writing here, for future reference.

I respect you and appreciate your input, even if we ultimately disagree, because I have never seen you do any of the above – but regardless of how this ends, I think a community that tolerates (i.e. ignores) abuses of power from devs who can not put their personal dislikes aside or close issues after being told that they didn't think something through is dysfunctional.

@v-rob
Copy link
Member

v-rob commented Sep 14, 2023

As for my actual opinion: I think the #1 reason for keeping TGA is backwards compatibility. We do have actual mods that use TGA for a number of legitimate purposes, and while TGA was technically undocumented for a number of years, so were PNG, JPG, and BMP, so modders cannot be criticized for that (although I would take a harder stance for using e.g. clearly undocumented APIs, where modders are at fault).

If there's a clear upgrade path for mods with suitable alternatives (as in, you don't have to write a PNG decoder from scratch, for instance), I believe the deprecation can be warrented. Otherwise, TGA is not a large maintenance burden, given that the code is small, isolated, and has never needed to be modified for the duration of its existence by us AFAIK.

All of which is to say, I would not oppose undeprecation, but I believe that deprecation is reasonable if the alternatives are viable (even if they are objectively worse in some metrics, like file size, despite further improvements).

@erlehmann
Copy link
Contributor Author

I believe that deprecation is reasonable if the alternatives are viable

The elephant in the room here is who decides what is actually a viable alternative.

If past performance predicts future performance, any “viable alternative” will be handled like:

  • The particles that are not batched and generate too many drawcalls.
  • minetest.compress() ZSTD support without custom dictionaries.
  • The dynamic media upgrade that introduced a race condition.
  • The PNG decoding with deliberate gamma breakage.
  • The bad PNG encoder that we all know about.
  • The very funny [png texture modifier.

In all these mostly texture-related cases, ease of implementation handily won against concerns regarding correctness, ease of use and/or performance – even when it was pointed out what could be done to address concerns engine-wise.

I do not expect a sudden jump in quality regarding API design or implementation quality here, given my experiences.

@erlehmann
Copy link
Contributor Author

@v-rob consider the following … 😉
gimp-and-a-tga-gz-file-highlighted

@celeron55
Copy link
Member

Gimp will open a tga.gz file, but it will not export it. Not even re-export after opening it. I guess they might consider that to be a bug though. The same does work for .xcf.gz. In fact, I generally use .xcf.bz2 for my gimp projects, to make them waste as little space as possible, and it works beautifully.

@sfan5
Copy link
Member

sfan5 commented Sep 17, 2023

I will not be making an argumentative contribution except say that I personally would still deprecate TGA.
However I wanted to clarify the following:

I was told that it is pretty rude how I dismiss people's opinions if they haven't done their research, but [...]

[...] pointing out when people say things that indicate they have no clue [...]

These statements exemplify the problem.
You do not even remotely consider the possibility that you are wrong. Opening discussions with "you are totally wrong and clueless" (arguments notwithstanding) will turn off any sane person from continuing a conversation. This is a consistent trend that you do not seem to see.

If past performance predicts future performance, any “viable alternative” will be handled like

When was the last time you attempted to help fix any of the mentioned problems?
You of course have no obligation to do so, but the current situation does not necessarily make the appearance of a net positive contributor.

I think a community that tolerates (i.e. ignores) abuses of power from devs who can not put their personal dislikes aside or close issues after being told that they didn't think something through is dysfunctional.

While it is correct that the issue you are referring to was not closed on technical grounds, I am not preventing you from requesting it to be reconsidered by other coredevs. Or requesting that I be reprimanded.
I'm sure you can imagine that likewise I might think that a project that tolerates continued abuse and (what essentially is) obstructive behavior towards development/developers dysfunctional. Ultimately only one of us will turn out correct on this.

@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 19, 2023

You do not even remotely consider the possibility that you are wrong. Opening discussions with "you are totally wrong and clueless" (arguments notwithstanding) will turn off any sane person from continuing a conversation. This is a consistent trend that you do not seem to see.

I do indeed consider the possibility I may be wrong. What I usually want to convey when I am rude like that is not “you are stupid and I am not”, but “what you claimed is a common claim by people who have not spent much time investigating the issue, I have had similar thoughts and they went away when I spent more time on details”. I concede that I am bad at delivering that message.

If you discussed some C or C++ code with a person and it came out that they think undefined behaviour and implementation-defined behaviour are the same (a common noob illusion), would you still take them as seriously as before? I would not, as the vehement claim that those are basically the same indicates a lack of domain knowledge.

When was the last time you attempted to help fix any of the mentioned problems?

  • Particles: Someone else tried fixing them, but their implementation was incomplete. It seems to me that the Irrlicht particle system with the better performance characteristics than MInetest's was deleted in IrrlichtMT PR #48, which also removed the built-in shadow rendering and TGA for the first time. That PR was approved by you – despite multiple people complaining about the latter.

  • minetest.compress() ZSTD support: I spent many hours on benchmarks and did a writeup on the issues. My warning that the proposed implementation was an API footgun was entirely ignored until very recently when @celeron55 explained to me that my warning was badly written (which I agree with) and @appgurueu told me that he had, indeed, not done the research (i.e. neither researched ZSTD design nor ran benchmarks himself) when creating the issue.

  • Dynamic media race conditions: I created an issue about it which was closed by you. I pointed out that making a blocking function non-blocking introduces race conditions and suggested several ways of not having them (e.g. blocking when there is no callback, not using the same function name for new business logic). I fixed the mcl_maps code for all three of MIneClone 2 / MIneClone 5 / Mineclonia when no one else would do it. Much later @appgurueu pointed out that I did not do enough research on the issue and my solution is probably still racy.

  • PNG decoding gamma issues: As a first step for gamma-correct rendering, I submitted a PR with 18 test nodes that could be used to diagnose gamma issues. It was decided that Minetest will never handle gamma correctly after I described what has to be done for that to happen, so I think I literally helped as much as I could.

  • Bad PNG encoding: I have probably spent several days overall on trying to figure out if it is worth to improve this. The result was that likely no amount of effort would be worth it for my use case … partially also because the quality of the exposed PNG encoding API suggests that Minetest's developers do not care much about dynamically generated textures. The rejection of the gamma test nodes further demotivated me. I chose to improve the tga_encoder mod – first for MineClone 2, later for other stuff – and regardless of what opinion you have on TGA as a format, that piece of software definitely works and is used.

  • The [png texture modifier: I claimed that the implementation was not a good idea on LANGSEC grounds and suggested multiple ways of inlining textures with an easier-to-validate grammar. I am pretty sure that I was correct … @rollerozxa told me that [png can be used to inline all texture file formats – which has turned out to be a blessing, as coras uc_signs and sitelen_pona_signs mods use that facility to inline TGA images.

@Montandalar
Copy link
Contributor

Casually observed, what I have seen from this discussion is an insistence that getting rid of TGA will help Minetest's "maintenance burden" somehow, or that TGA is inherently archaic and to be deprecated. If I look at the issue list, what I see is a list of TGA improvements from erlehmann, and some attempts to get rid of TGA, with the thinking from hecks that "since we never explicity supported it, get rid of it". Well, now we know we have an established base of users of the format as detailed in this thread's OP.

What harms do the core devs see in keeping the TGA code? Is there a planned OpenGL rewrite and further move away from irrlichtMt that will require a new code for TGA, or do the devs just not want to have to respond to any bug reports that may pop up? They seem few to me. TGA gets few (direct, not Mineclone2) users, it gets few bug reports. Surely the format can just stay "on ice"?

@sfan5
Copy link
Member

sfan5 commented Sep 19, 2023

Particles, minetest.compress() ZSTD support, Dynamic media race conditions, [...]

You don't seem to understand. I am talking about finished implementations, not proposals, discussions or ideas.

@erlehmann
Copy link
Contributor Author

erlehmann commented Sep 19, 2023

Particles, minetest.compress() ZSTD support, Dynamic media race conditions, [...]

You don't seem to understand. I am talking about finished implementations, not proposals, discussions or ideas.

In cases where after spending time on the issue my proposal is ”do nothing instead” (e.g. ZSTD in minetest.compress() or not deprecating TGA) I can not possibly offer an implementation. In other cases for which solutions are not trivial, I am convinced that not discussing what approach to use would waste my and the coredevs' time. I am very glad that I did not try to fix the gamma-incorrect rendering or wrote engine code to mitigate the race conditions, as those proposals would have been rejected anyway.

I do think now that I should …

  • … at least provide proof-of-concept PRs for relatively simple stuff where a “common sense” intuition is that my approach is too much work (though my experience with build system issues has proven that this is not a guarantee to convince people).
  • … explicitly point out that when I submit test cases that demonstrate bugs that I want to help fixing those bugs.

Btw: I hope my analysis in minetest/irrlicht#236 proves to you and others that I want to help (not hinder) development.

@servantoftestator
Copy link

I tried searching for a better alternative to TGA that would justify removing it https://letmegooglethat.com/?q=truevision+graphics+adapter+competitor+successor . It doesn't seem that anybody has developed a better format for uncompressed lossless files. Google did some work some years back on compressed lossless files and lossy files with webp. But the king of uncompressed is still TGA. According to google its used in a lot of game engines so it doesn't really make sense to get rid of it just yet from a social aspect. And erlehmann, TGA as a format is highly scrutinized in its implementations due to age and ability to be read and wrote to along with high availability. Which is to say its insecure to some extent. I see that you've attempted to improve irrlicht's implementation of TGA but what about another direction instead, depreciate the current implementation of TGA and investigate a different implementation that someone else maintains for future maintenance burden reduction that could be merged instead? E.g something like https://github.com/devprofile98/targa-rs but c++. Or maybe having more then two languages in the engine codebase wouldn't be a problem?

@numberZero
Copy link
Contributor

is highly scrutinized <...> Which is to say its insecure to some extent.

Saying a format is insecure is meaningless. It’s not an encryption algorithm or anything. And in those, the most secure are old ones that are still not cracked despite lots of time and effort put into that, while new stuff is considered as unknown quality.

depreciate the current implementation of TGA and investigate a different implementation that someone else maintains

It may be SDL_image. But unless that would be used for other image formats as well trying to bring an external library for such a simple thing as TGA doesn’t make any sense. This thing is so tiny even rewriting it all wouldn’t be a big problem. And has less bugs than e.g. the BMP decoder.

maintenance burden

A self-inflicted thing, largely. But that’s another story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature. @ Script API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants