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

Foliage using allfaces_optional becomes too dark with distance (most leaf texture transparent pixels need improved colouring) #2801

Closed
TheTermos opened this issue Dec 31, 2020 · 15 comments

Comments

@TheTermos
Copy link

This is the effect of transparent pixels' color being neglected and left at default black, which gets mixed in whenever a texture gets downscaled.
The fix is as easy as changing the color of transparent pixels to one that fits the context.

This affects tree leaves the most, I could edit the textures as I'm working on another project involving textures,
just please let me know somebody if it's going to get any attention given the whole situation with MTG.

left is now, right is fixed:
fade2black

@SmallJoker
Copy link
Member

Isn't this an engine issue?

@TheTermos
Copy link
Author

I'm not sure tbh, but I believe mipmapping is a fairly low level hardware functionality these days. I wouldn't expect there to be that much control over how it'done. We should ask somebody well versed in modern OpenGL.

One possible engine issue is that I couldn't get MT to correctly apply the color of transparent pixels of png texture in indexed mode (it seems to always pick a specific pallete entry number regardless the actual color of transparent pixels). But RGB is fine and most MTG leaf textures are RGB anyways.

But there's also the 'opaque leaves' mode, and leaves engraved in pitch black blocks don't look good either, so fixing textures is probably the easiest and correct solution.

@paramat
Copy link
Contributor

paramat commented Jan 3, 2021

This is the effect of transparent pixels' color being neglected and left at default black

But there's also the 'opaque leaves' mode, and leaves engraved in pitch black blocks don't look good

These colours are not neglected and are not black.

which gets mixed in whenever a texture gets downscaled.

This is an engine issue, maybe an engine issue should be opened for this specific problem?

The fix is as easy as changing the color of transparent pixels to one that fits the context.

The transparent pixels of all leaves textures have already been set to 'darker leaf' / 'dark inside of leaf node' colours to make 'opaque leaves' mode look reasonable, i did most of this work.
If these pixels are made too light in colour, or similar to 'surface leaf' colour as in your proposed fix, the 'surface leaves' in opaque mode lose their edge definition, contrast is necessary, this is why the transparent pixels are darker versions of the surface leaf colour.

So i disagree with making the transparent pixels similar to 'surface leaf' colour as in your fix.
Perhaps a compromise can be found by making the transparent pixels slightly lighter. This has to be balanced with opaque mode appearance. I agree the trees get too dark at a distance currently.

One possible engine issue is that I couldn't get MT to correctly apply the color of transparent pixels of png texture in indexed mode (it seems to always pick a specific pallete entry number regardless the actual color of transparent pixels). But RGB is fine and most MTG leaf textures are RGB anyways.

Yes this is known. 'allfaces optional' textures are documented as needing to be RGBA for this reason. All MTG 'allfaces optional' leaves have RGBA textures.

@paramat paramat changed the title foliage using allfaces_optional fade to black with distance foliage using allfaces_optional becomes too dark with distance Jan 3, 2021
@paramat paramat changed the title foliage using allfaces_optional becomes too dark with distance Foliage using allfaces_optional becomes too dark with distance Jan 3, 2021
@paramat
Copy link
Contributor

paramat commented Jan 3, 2021

so fixing textures is probably the easiest and correct solution.

It is the easiest, but it is not the 'correct' solution. Fixing this in the engine would be the ideal solution so that transparent pixel colours and opaque leaf appearance do not have to be compromised.

First we should find out if an engine solution is possible. If not, as seems likely, then changing the MTG textures can be considered (and i think can count as 'essential maintenence').

Acacia leaves do not need changing as they are so sparse. I made that texture and put a lot of work into the transparent pixels of those. So i disapprove of it being changed.

@paramat
Copy link
Contributor

paramat commented Jan 3, 2021

The transparent pixel colours in the leaves textures are (for at least some of the textures, probably not all) not a single colour.
A 'dark leaves background' texture has been created by overlaying the 'non-transparent pixel leaf texture' over itself a few times with different position offsets until no transparent pixels remain, that is then darkened and placed behind the 'non-transparent pixel leaf texture' as transparent pixels to suggest layers of darker interior leaves.
Quite a lot of work.
Any new textures submitted should do the same to preserve the quality, a single colour for all transparent pixels does not look good.

@TheTermos
Copy link
Author

The transparent pixels of all leaves textures have already been set to 'darker leaf' / 'dark inside of leaf node' colours to make 'opaque leaves' mode look reasonable, i did most of this work.

Did you?
Sorry but it looks like transparent pixels are still black, you might want to check again.

So i disagree with making the transparent pixels similar to 'surface leaf' colour as in your fix.

Wait, I haven't mentioned any specific colors, so there's nothing for you to disagree with yet.

While we're at it, the color should be close to the average of all opaque pixels, otherwise with mipmapping on, the transition is abrupt, especially jarring when player is in motion. And between 'fancy' and opaque leaves, you have to favor the former, because opaque mode is for performance, not looks, and it looks kinda bad whatever you do.

Fixing this in the engine would be the ideal solution so that transparent pixel colours and opaque leaf appearance do not have to be compromised.

Yeah, that's unlikely to happen, meanwhile the textures in question were last edited 4 years ago.

It's not only allfaces, for example go to the savanna biome and watch how half the dry grass fades to black, while the other half to white (they're indexed)
It's baffling how hardly anyone even notices these things here.

@paramat
Copy link
Contributor

paramat commented Jan 7, 2021

Sorry, i was fairly sure the transparent pixels for tree leaves had been well coloured. But i checked and apple, aspen and jungle leaves all have black pixels, which is unacceptable.
Blueberry bush leaves have very dark grey pixels, also not good.
Acacia and pine have dark green pixels so have been previously attended to.
Details below.
I wonder if texture compression messed these up, but maybe not.

Acacia has dark green pixels, all of one colour, which is odd because i seem to remember adding layers of darkened leaves behind the surface leaves (perhaps i started this task then gave up or decided against it).
These pixels have alpha 2 instead of 0, which needs correcting.
These pixels could be made a little lighter than they are now while still having essential significant contrast.
These pixels should have varying colours because the gaps are large.

Pine needles are fine and have necessarily dark pixels (because the surface needles are already quite dark) of varying colours. This texture does not need changing.

Apple, aspen and jungle leaves have black pixels and so need improvement. They have large gaps so the pixels need colour variation.

Blueberry bush leaves have dark grey pixels, which should at least be dark green instead.
The texture has small gaps, and has dark pixels in the surface leaves, so the transparent pixels need to be very dark green, and one colour is sufficient.

//////////////////////////////////////////////

To be clear, the suggested changes above are only to improve the opaque mode appearance. I disapprove of compromising the opaque mode appearance to alter the way leaves become darker with distance.
Leaves becoming darker with distance is much less of a problem than bad-looking opaque leaves with little or no contrast, definition or depth.

As this issue reports, with fancy mode leaves and mip-mapping on, distant leaves have an appearance determined by all their pixel colours, so have the colouring of opaque mode leaves.

The opaque mode colouring is valid. The appearance of leaves changes from fancy mode colouring to opaque mode colouring at a distance. Both colourings are equally valid, there is no invalid colouring. So this change of colouring with distance is not really a problem.

Ideally of course, in fancy mode the engine would give distant leaves fancy mode colouring. But if this is not possible as we suspect, i think it is not justified to compromise the opaque mode appearance.

Luckily, improving the textures as described above will result in less dark transparent pixels, which will have the side effect of slightly reducing the darkening with distance.

@paramat paramat changed the title Foliage using allfaces_optional becomes too dark with distance Foliage using allfaces_optional becomes too dark with distance (most leaf texture transparent pixels need improved colouring) Jan 7, 2021
@TheTermos
Copy link
Author

The opaque mode colouring is valid. The appearance of leaves changes from fancy mode colouring to opaque mode colouring at a distance. Both colourings are equally valid, there is no invalid colouring. So this change of colouring with distance is not really a problem.

Not really a problem? This is what happens when transparent pixels' color deviates from the average too much (MTG 5.3.0).
mip

Opaque mode is ugly ass and hardly used. That the appearance of leaves changes from fancy to opaque at distance is irrelevant because the textures are already downscaled at that distance.

The correct way to go about this is making sure that the color transition from one level to another in fancy mipmapped is smooth, and this should be the constraint for the opaque mode.
You clearly have different ideas so I'm leaving you to it, I'm sure you're more than capable of continuing to make MTG just as successful as it has been up to this point.
Editing these textures was less work for me than talking to you here, so let's hope this doesn't take you another 4 years.

@SmallJoker
Copy link
Member

SmallJoker commented Jan 15, 2021

What happens if you run optipng -o7 or pngcrush on the edited textures? That's usually done to keep textures small, but might also discard transparent pixel information which would ruin this fix.
I thought there's mipmapping code -> related luanti-org/luanti#6917

@TheTermos
Copy link
Author

What happens if you run optipng -o7 or pngcrush on the edited textures?

Ran a win32 version of pngcrush on my copy of default_leaves.png - 62% size reduction, background color still intact.
Makes sense, as they claim the procedure is losless.

I thought there's mipmapping code -> related minetest/minetest#6917

Yes, mip levels can be supplied directly, but that feels so 20th century, and offloading GPU work to CPU might not be a good idea for MT.

@TheTermos
Copy link
Author

Forgot to add, pngcrush did not convert it to indexed, it's still RGB

@ExeVirus
Copy link
Contributor

ExeVirus commented Feb 2, 2021

TheTermos:

I know your pain, trust me. And don't worry capable people (probably me at some point) will tackle the mipmap issue eventually. As for compression, I have a compression commit pending that fully respects RGBA textures: #2808 that is likely to be accepted. This uses optipng, which does the same as pngcrush and about 20 other png compression utilities.

The resulting differences in size between going to RBGA indexed (which pngcrush does by default, and indexed still means RGBA, it's just a lookup table value, long story) and RGBA no-index results in only about ~11.2 KB in savings after my commit is applied. While I'd love to squeeze another 11.2KB out, the safe compression commit squeezes out 93KB of the original ~242KB, which is most of the possible savings.

Long story short, there are much better ways to create savings for media sizes in minetest, including but not limited to: helping us get opus support. Helping get .webp support (which saves another 30 KB or so). Helping to make .obj's compressed with gzip by default. Helping us to have zstandard as our compression library, adding fluidsynth or other midi-style music support, etc.

You get the idea, there's bigger fish to fry.

@TheTermos
Copy link
Author

ExeVirus, the topic of compression sure is fascinating, but it's not particularly relevant to this issue.
Transparent pixels' color should be corrected, regardless if the textures are being compressed or not.

Anyway, this looks like another 30min easy fix that turns into a lifelong endeavor.

@appgurueu appgurueu added the Bug label Apr 4, 2022
@appgurueu
Copy link
Contributor

What happens if you run optipng -o7 or pngcrush on the edited textures?

To add to what TheTermos pointed out already, this patch suggests that OptiPNG keeps them intact. A post in that thread suggests that pngcrush doesn't strip the colors of pixels with an alpha of zero either. I haven't tested this though.

While the root cause for this indeed is an engine issue, I believe we may temporarily fix this in the way @TheTermos has described until it's fixed on the engine side of things. This issue stems from GL not weighing (or MT/Irrlicht not instructing it to?) pixels by alpha when filtering for mipmapping, and there might even be some graphic cards not even supporting such weighted filtering, possibly making it necessary to change the textures.

Transparent pixels' color should be corrected, regardless if the textures are being compressed or not.

Agreed. Should this be automatized (e.g. floodfill + avg from opaque colors)?

@sfan5
Copy link
Contributor

sfan5 commented Apr 4, 2022

Transparent pixels' color should be corrected, regardless if the textures are being compressed or not.

Agreed. Should this be automatized (e.g. floodfill + avg from opaque colors)?

The engine already does this luanti-org/luanti#11145

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

6 participants