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

Rendering improvements related to liquids #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

grorp
Copy link

@grorp grorp commented May 16, 2023

Instead of using of the whole texture, only the first frame of animated tiles is used. This for { type = "vertical_frames" } tile animations only.

Changes this
screenshot before
into this
screenshot after

I tried to modify this Minetest client in order to learn some Rust and then made a pull request because why not.

@grorp grorp force-pushed the animation-first-frame branch from 00eefb4 to e846602 Compare May 17, 2023 18:20
@grorp
Copy link
Author

grorp commented May 17, 2023

I've added more more rendering improvements related to liquids to this PR:

  • Flowing liquids now only use the first two textures from special_tiles, as they should.
  • Faces of flowing liquids are now, like those of liquid sources, culled if their neighbor is a cube or the same node.
  • Faces of both liquid sources and flowing liquids are now also culled if their neighbor is their "alternative form", i.e. their liquid_alternative_flowing or liquid_alternative_source.

Another comparison:

before:
Bildschirmfoto vom 2023-05-17 20-16-18

after:
Bildschirmfoto vom 2023-05-17 20-16-58

@grorp grorp changed the title Use the first frame of animated tiles Rendering improvements related to liquids May 17, 2023
@grorp
Copy link
Author

grorp commented May 19, 2023

And another comparison:

before:
screenshot before

after:
screenshot after

@LizzyFleckenstein03
Copy link
Contributor

Hi, thanks for your PR, I'm sorry I only noticed it now.

This will be merged soon-ish. I'm currently working with luatic on texture modifiers, and I want to use texture modifiers to extract the verticalframe from the texture. This way I can rely on the same caching mechanism for all sorts of modifications that are done to an image, so that will be done first.

Also note that I have previously determined HashMap lookups to be a major bottleneck in the create_mesh function (flamegraph revealed it), hence the weird [Option<Box<NodeDef>>; u16::MAX as usize + 1] construct. This means that the node_names_to_ids will be replaced by an array lookup as well.

You don't need to fix or update this PR, I will do it when I merge it. The best way people can currently help me is by researching and documenting minetest behavior, so this PR is helpful, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants