OK, I fixed it, tested the fix, and pushed the change. Baking with a Y- tangent space swizzle should work now if you don't flip the green channel, and baking with a Y+ tangent space swizzle should work now if you do flip the green channel. It was a super easy fix
So here we have the world normal buffer for the static mesh (on the left) and rigged mesh (on the right) using an xNormal bake from a high poly rounded-edge cube.
Spot on
I'll give it a shot with mirrored meshes in a bit.
However, the Y/green channel used to have to be flipped and now it doesn't.
I'm not sure which is preferable for Epic. Really, they should fix all their normal maps and adopt this new version - have it done correctly - but that might piss off a lot of people :P
Can't comment on tangent changes but if it requires flipping green channel of normal map that will have to change because there's too much content that would have to be updated.
Although, playing with it some more, it seems that your implementation for static meshes is actually better than the implementation for the rigged meshes.
They're still showing lighting issues where the static ones are exactly right.
I wonder if it's worth throwing the skeletal meshes through this, too. I'll have a play.
Here's a side-by-side of the 4 different cubes (cube/mirrored static/skeletal). There is a single directional that is facing exactly along the axis.
Yeah, it could be worth for skeletal meshes too. Just because I didn't notice any problems with the implementation for skeletal meshes doesn't mean that the problems don't exist. I'll take a look.
Looks like it'll need a separate interface with MikkTSpace, it's not the same data that's passed to the method BuildSkeletalMesh and it also includes a lot of stuff for weights and chunks (material IDs) that isn't a problem with the static stuff.
Not going to have time to look further into it tonight
Figured out the GitHub access thing, is there anyway to thumbs up or recommend Epic look at the pull request? I'm sure they get a bunch, I'd just like to see this one get pushed through quickly.
Jed: Hmm, I'll give it a try when I get a chance. But looking through their code, it's much the same bag as the static mesh stuff - i.e. not actually MikkTSpace.
Files are there with the xNormal bakes for the regular and mirrored versions (both the rigged and non-rigged variants use the same textures as each other).
Also included the cage and high poly FBX files if you want to bake in xNormal.
I'm half-way through implementing it but I'm having issues with UV seams I think... might need to look deeper into how the data is laid out for the mesh, could be it's not quite what I think it is.
Ah, yeah, I was wrong about how the data was being stored. I've fixed that up now and skeletal meshes have proper MikkTSpace implementation - those shading errors are gone
Should I send the files over to you so you can implement it into your pull request? Makes sense to keep it all in the same place.
Sweet, how do you want to get your code integrated then? Maybe if you make a pull request against my fork I can integrate the changes and they will probably show up in the pull request that has the fix for static meshes.
Ah, yeah, I was wrong about how the data was being stored. I've fixed that up now and skeletal meshes have proper MikkTSpace implementation - those shading errors are gone
Should I send the files over to you so you can implement it into your pull request? Makes sense to keep it all in the same place.
Awesome, nice work, Farfarer! You are a valuable asset to the community. And you too, JedTheKrampus! Thanks for looking into this. And thanks, Warren, for bringing this to the attention of the engineers.
Updated pull request is up. Includes reference MikkTSpace for skeletal meshes, thanks to Farfarer! Super fancy. If you got da skilz to do da buildz, please give it the old once-over with your stress test meshes, imported as both static or skeletal, especially if you work at Epic and can help get this merged (looking at you WarrenM.) If you can get UE4 to crash anymore than it usually does, please let me know! (but that shouldn't be the case)
Not sure, I'm guessing they know of its existence. It'll need to be tested on their end and possibly checked out by legal team over its usage rights. It's going to make quite a change to all of the mesh assets in a game that use calculated tangents, so it's no small addition.
My thought was to post it there so hopefully more people would show interest in the feature. Even if it doesn't get implemented fully, I'd really like this to be a check box in project settings.
Don't worry, it takes them a while to get to pull requests. These guys are awfully busy. If you make a thread I'd do it in Content Creation to discuss the merits of this solution.
I have a reasonable suspicion that they're working on getting it integrated. I personally hope they hurry up, as I can't post any more pull requests until they get this one in.
They usually communicate what they're doing though. If they get to it they let you know in the pull requests comments. But maybe this one is different.
I have several hypotheses for the lack of communication.
- It's a bigger-than-average pull request, so they haven't decided how or if they're going to integrate it yet and don't want to make any promises here that they won't be able to keep if something happens.
- Normals can be a hot issue with a lot of potential for controversy. Artists tend to get angry when they can't get their normal maps working correctly, leading to incivility on the discussion board and ill will toward Epic. They, of course, wish to defer this ill will as long as they can, so they wish to avoid discussing the matter in public before they have good news.
- The integration into the build system is a little bit dodgy in my original pull request. It's overkill to ship static libraries for MikkTSpace as a build dependency, but just including MikkTSpace.c into the source tree, as would be more sensible, breaks precompiled headers in Visual Studio. Maybe a patch to UnrealBuildTool is required to integrate the pull request properly, requiring loads of discussion during a boring change control meeting, delaying integration of the pull request. I am not very familiar with the build system myself, so I wasn't as easily able to make it do what I really wanted it to.
- The rendering team is likely made up of strong, silent and rugged men and/or women who eat integrals for breakfast and solder homebrew GPUs for fun after they get back from their hunting trip where they wrestled a bear with their bear hands and won, leaving the bear's pride obliterated but its life still intact. In my mind this is the most likely explanation.
Personally I would like to see some public communication as much as you would. I would really like to see an easy 100% tangent space sync to MikkTSpace for Unreal Engine, as it means less time reconfiguring UVs to hide shading errors from split tangents and normal map mirroring, and more time making art, which is really what it's all about. So, rendering team, if you're reading this: less bear wrestling, more communication please!
Well again, I think everything is alright and there will be communication soon. For my own pull request (which was a ridiculously small thing compared to this) I also waited about 2 weeks before anyone from Epic even got to it. They are busy people.
Ok I take it back Just had a (again very minor) change integrated in just 2 days. So it's very probable they've at least seen this but decided not to comment for some reason.
Of course it may just be that the graphics programmers who need to check this are super busy.
I can give them a bit of a pass since Thanksgiving was last week, but to be quite frank there's only so much time that I can spend waiting. I need them to merge the pull request already so that I can put it on my resume before I graduate from university on the 19th and work on finding a job. It would be nice if I worked at Epic and could assert some dominance internally to ameliorate the many problems and deficiencies of the engine, but the fact is that I'm a student right now and I have to do student things as well.
If there's a problem with the patch I sure wish they would tell me.
Replies
Solid work though, dude, very much appreciated!
So here we have the world normal buffer for the static mesh (on the left) and rigged mesh (on the right) using an xNormal bake from a high poly rounded-edge cube.
Spot on
I'll give it a shot with mirrored meshes in a bit.
Can't comment on tangent changes but if it requires flipping green channel of normal map that will have to change because there's too much content that would have to be updated.
Edit> looks like I missed your updates
https://github.com/EpicGames/UnrealEngine/pull/561
They're still showing lighting issues where the static ones are exactly right.
I wonder if it's worth throwing the skeletal meshes through this, too. I'll have a play.
Here's a side-by-side of the 4 different cubes (cube/mirrored static/skeletal). There is a single directional that is facing exactly along the axis.
I'm getting a 404 error
Not going to have time to look further into it tonight
Zac: Not that I know of.
http://www.farfarer.com/temp/UDKMikkTSpaceTest.zip
Files are there with the xNormal bakes for the regular and mirrored versions (both the rigged and non-rigged variants use the same textures as each other).
Also included the cage and high poly FBX files if you want to bake in xNormal.
I'm half-way through implementing it but I'm having issues with UV seams I think... might need to look deeper into how the data is laid out for the mesh, could be it's not quite what I think it is.
Should I send the files over to you so you can implement it into your pull request? Makes sense to keep it all in the same place.
You got an email address I can just send the updated MeshUtilities.cpp to and you can just add it as a revision to your pull request?
Awesome, nice work, Farfarer! You are a valuable asset to the community. And you too, JedTheKrampus! Thanks for looking into this. And thanks, Warren, for bringing this to the attention of the engineers.
Pull request:
https://github.com/EpicGames/UnrealEngine/pull/561
- It's a bigger-than-average pull request, so they haven't decided how or if they're going to integrate it yet and don't want to make any promises here that they won't be able to keep if something happens.
- Normals can be a hot issue with a lot of potential for controversy. Artists tend to get angry when they can't get their normal maps working correctly, leading to incivility on the discussion board and ill will toward Epic. They, of course, wish to defer this ill will as long as they can, so they wish to avoid discussing the matter in public before they have good news.
- The integration into the build system is a little bit dodgy in my original pull request. It's overkill to ship static libraries for MikkTSpace as a build dependency, but just including MikkTSpace.c into the source tree, as would be more sensible, breaks precompiled headers in Visual Studio. Maybe a patch to UnrealBuildTool is required to integrate the pull request properly, requiring loads of discussion during a boring change control meeting, delaying integration of the pull request. I am not very familiar with the build system myself, so I wasn't as easily able to make it do what I really wanted it to.
- The rendering team is likely made up of strong, silent and rugged men and/or women who eat integrals for breakfast and solder homebrew GPUs for fun after they get back from their hunting trip where they wrestled a bear with their bear hands and won, leaving the bear's pride obliterated but its life still intact. In my mind this is the most likely explanation.
Personally I would like to see some public communication as much as you would. I would really like to see an easy 100% tangent space sync to MikkTSpace for Unreal Engine, as it means less time reconfiguring UVs to hide shading errors from split tangents and normal map mirroring, and more time making art, which is really what it's all about. So, rendering team, if you're reading this: less bear wrestling, more communication please!
Ok I take it back
Of course it may just be that the graphics programmers who need to check this are super busy.
Guessing it's missed the 4.6 boat, but better late than never
I can give them a bit of a pass since Thanksgiving was last week, but to be quite frank there's only so much time that I can spend waiting. I need them to merge the pull request already so that I can put it on my resume before I graduate from university on the 19th and work on finding a job. It would be nice if I worked at Epic and could assert some dominance internally to ameliorate the many problems and deficiencies of the engine, but the fact is that I'm a student right now and I have to do student things as well.
If there's a problem with the patch I sure wish they would tell me.