-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix rendering of translucent items in inventory & when wielded #14701
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I've been trying to get through the In both cases (wield & inventory), the mesh comes from Now you've made the overwriting work correctly, but the transparency material is still being set somewhere in the mesh generation code too. If possible, I think it would be preferable to make the mesh generation code set the transparency material correctly, getting rid of the overwriting. |
Awesome! These broken translucent inventory images for the transparency test in DevTest have bothered me for a long time. Thanks for fixing this. I found a regression: When you wield Water sources turn semi-transparent, but the flowing water doesn't. Seems to be only the default flowing water tho. Maybe this is something in the node definition (so not your fault)? The rest seems fine. I tested wielding all the items (tip: I noticed there is no DevTest test item for testing the case of a |
This comment was marked as outdated.
This comment was marked as outdated.
Thanks for finding this. Fixed in 8c11c12:
Indeed this is a devtest bug; the flowing water definition is missing a
Added in a509aad, works as expected: |
Now every item glow in the wieldhand and as dropped item. Test the wield image at night/darkness and you'll see. |
I'm not sure I can reproduce. This is after vs. Tested with ed11b33. I also reviewed the code - I shouldn't haven't touched any |
Can reproduce with The |
Posting this thing here too: SummaryThe mighty
|
Edit: I decided to leave the overwriting for plantlike drawtypes in for alpha blending consistency (if a plantlike node has opaque or clipped alpha, the extrusion mesh should be consistent with that). The alternative would be to pass the appropriate material type & param to
You're right, I should try to use this consistently for nodes.
Pretty much does a subset of the things
I wrongly assumed that this was obsolete after my refactoring, which introduced the regression. I still find it weird that we first give our materials to our shader to then get an Irrlicht material type (?) back, rather than having our shader deal properly with the material type it is given. |
I believe it returns a "shader material" with an index past the builtin materials and that's how the shader is applied. |
Yes. This is quite a dirty hack IMO, but I won't fix it here. Looking at the enum, this is unexpected (though Something like Anyways, I think as far as this fix goes, this PR should be okay now? Please check whether I've missed anything. |
@@ -41,6 +41,56 @@ with this program; if not, write to the Free Software Foundation, Inc., | |||
#define MIN_EXTRUSION_MESH_RESOLUTION 16 | |||
#define MAX_EXTRUSION_MESH_RESOLUTION 512 | |||
|
|||
/*! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I just moved this around because I made it static
.)
@@ -193,10 +243,55 @@ class ExtrusionMeshCache: public IReferenceCounted | |||
|
|||
static ExtrusionMeshCache *g_extrusion_mesh_cache = nullptr; | |||
|
|||
static scene::SMesh *getExtrudedMesh(ITextureSource *tsrc, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Same here)
Indeed.
This would have unneeded overhead and make few sense given internal handling is often identical. |
New summary
|
Closes #5461. Ready for review.
How to test
Look at the three marbles, both in the inventory and as wield items:
Also look at the alpha test node with alpha = 64. It should be visible, both in your inventory and when wielded. It should look like this:
Cycle through the devtest items and verify that they look right; also look at some simple wield items, like pickaxes, to make sure that there are no regressions.
Furthermore, enable
inventory_items_animations
and check that the spinny items look as expected.Relevant documentation: https://irrlicht.sourceforge.io/docu/namespaceirr_1_1video.html#ac8e9b6c66f7cebabd1a6d30cbc5430f1ac08aa3715ad41281472202107a81f736