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

Allow Irrlicht caching of animated meshes #6723

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@lhofhansl
Contributor

lhofhansl commented Dec 1, 2017

Possible solution for #6676 !

Instead of setting the color of each vertex (which is shared between instances of the same animated mesh), set emissive color instead, which is unique per instance of the mesh.

That way we can use Irrlicht's mesh cache.

This is for testing. Please have a look at this and test it out.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 1, 2017

Contributor

Note that when we go real lighting, this will have to be thought through again.

Together with #6721 and #6667 this quite improves the game experience.

Contributor

lhofhansl commented Dec 1, 2017

Note that when we go real lighting, this will have to be thought through again.

Together with #6721 and #6667 this quite improves the game experience.

mesh->grab();
RenderingEngine::get_mesh_cache()->removeMesh(mesh);

This comment has been minimized.

@lhofhansl

lhofhansl Dec 1, 2017

Contributor

Note that this can potentially be improved further. MT's mesh data cache is no longer needed.
But I'd like to leave this for another PR.

Update: Nope. See changes below. It's only selectively using the Irrlicht cache for animated meshes now.

@lhofhansl

lhofhansl Dec 1, 2017

Contributor

Note that this can potentially be improved further. MT's mesh data cache is no longer needed.
But I'd like to leave this for another PR.

Update: Nope. See changes below. It's only selectively using the Irrlicht cache for animated meshes now.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 1, 2017

Contributor

I played around with this. Used torches and other light sources, day and night. This seems to work fine in all scenarios.
Even tried the torches mod that allows for a wielded torch, all the active objects were lit correctly.

I think we can remove the WIP label.

Contributor

lhofhansl commented Dec 1, 2017

I played around with this. Used torches and other light sources, day and night. This seems to work fine in all scenarios.
Even tried the torches mod that allows for a wielded torch, all the active objects were lit correctly.

I think we can remove the WIP label.

@lhofhansl lhofhansl added Testing needed and removed WIP labels Dec 1, 2017

@Fixer-007

This comment has been minimized.

Show comment
Hide comment
@Fixer-007

Fixer-007 Dec 2, 2017

Contributor

In singleplayer, problems with rendering of gates (no rendering in world, broken icon):
default
In singleplayer, install homedecor mod and observe strange object enlarging like this:
default
In multiplayer, join Time Travel Server to observe smth like this:
default

Contributor

Fixer-007 commented Dec 2, 2017

In singleplayer, problems with rendering of gates (no rendering in world, broken icon):
default
In singleplayer, install homedecor mod and observe strange object enlarging like this:
default
In multiplayer, join Time Travel Server to observe smth like this:
default

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 2, 2017

Contributor

Thanks @Fixer-007 will look into these.

Strange on the doors, and home decor items, I'm not changing any of the geometries.
How is the last scene supposed to look?

Contributor

lhofhansl commented Dec 2, 2017

Thanks @Fixer-007 will look into these.

Strange on the doors, and home decor items, I'm not changing any of the geometries.
How is the last scene supposed to look?

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 2, 2017

Contributor

How about this @Fixer-007 ?
Generally changing client::getMesh(...) was too radical. Changed it such that the default is not using the Irrlicht mesh cache; and animated meshes override this. This fixes the door issue in the inventory. Will also test home decor now.

Update: Yep, also fixes the home decor issue.

It was simply a mistake to always use the cache.

Contributor

lhofhansl commented Dec 2, 2017

How about this @Fixer-007 ?
Generally changing client::getMesh(...) was too radical. Changed it such that the default is not using the Irrlicht mesh cache; and animated meshes override this. This fixes the door issue in the inventory. Will also test home decor now.

Update: Yep, also fixes the home decor issue.

It was simply a mistake to always use the cache.

@Fixer-007

This comment has been minimized.

Show comment
Hide comment
@Fixer-007

Fixer-007 Dec 2, 2017

Contributor

Can confirm, bug was fixed.

Contributor

Fixer-007 commented Dec 2, 2017

Can confirm, bug was fixed.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 4, 2017

Contributor

Anything else I need to do here? :)
I ran this on different types of machines, with various mods, SinglePlayer, real server, etc.

Contributor

lhofhansl commented Dec 4, 2017

Anything else I need to do here? :)
I ran this on different types of machines, with various mods, SinglePlayer, real server, etc.

@sfan5

This comment has been minimized.

Show comment
Hide comment
@sfan5

sfan5 Dec 4, 2017

Member

Tested and works 👍

Member

sfan5 commented Dec 4, 2017

Tested and works 👍

@sfan5 sfan5 added the One approval label Dec 4, 2017

@Fixer-007

This comment has been minimized.

Show comment
Hide comment
@Fixer-007

Fixer-007 Dec 4, 2017

Contributor

Found another bug, it seems, player skin is not rendered on jordach.net port 30000, not perfectly sure,
c0dd4ea had no such bug, but ab947bd with this PR - does.

Contributor

Fixer-007 commented Dec 4, 2017

Found another bug, it seems, player skin is not rendered on jordach.net port 30000, not perfectly sure,
c0dd4ea had no such bug, but ab947bd with this PR - does.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 4, 2017

Contributor

Thanks @sfan5

This should not affect skins. I will of course double check, before I merge.

Thanks for the thorough testing @Fixer-007 .

Contributor

lhofhansl commented Dec 4, 2017

Thanks @sfan5

This should not affect skins. I will of course double check, before I merge.

Thanks for the thorough testing @Fixer-007 .

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 5, 2017

Contributor

@Fixer-007 Do you know which mod that is?

Contributor

lhofhansl commented Dec 5, 2017

@Fixer-007 Do you know which mod that is?

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 5, 2017

Contributor

I tried the 3d-armor mod and wardrobe mod. All works correctly.

I can only reproduce that with that server. :(

GenericCAO is also used for players, I think I should disable Irrlicht's meshcaching for players.
Will verify that theory now. :)

Contributor

lhofhansl commented Dec 5, 2017

I tried the 3d-armor mod and wardrobe mod. All works correctly.

I can only reproduce that with that server. :(

GenericCAO is also used for players, I think I should disable Irrlicht's meshcaching for players.
Will verify that theory now. :)

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 5, 2017

Contributor

Updated:
OK... Found it. The vertices were set to transparent. Simply setting the (unused) vertex color when the mesh is added to the scene to make sure the alpha is set. That fixes the issues. Player meshes can still be cached.

Contributor

lhofhansl commented Dec 5, 2017

Updated:
OK... Found it. The vertices were set to transparent. Simply setting the (unused) vertex color when the mesh is added to the scene to make sure the alpha is set. That fixes the issues. Player meshes can still be cached.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl

lhofhansl Dec 5, 2017

Contributor

Here's the change. No model should have vertexes with alpha set to 0 (transparent), but since it appears to be case here, this now making sure an alpha is set.

I'll merge this later tonight unless I see more comments.

Contributor

lhofhansl commented Dec 5, 2017

Here's the change. No model should have vertexes with alpha set to 0 (transparent), but since it appears to be case here, this now making sure an alpha is set.

I'll merge this later tonight unless I see more comments.

@lhofhansl

This comment has been minimized.

Show comment
Hide comment
@lhofhansl
Contributor

lhofhansl commented Dec 5, 2017

@lhofhansl lhofhansl closed this Dec 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment