Download

Getting Rid of FPackedNormal or raising its accuracy

Hello guys,

Not sure if this is better placed in Engine Source & GitHub but as it concerns mainly rendering, I just put it in this forum section.

So the problem is, that with dense meshes, the engine “optimizes” the meshes render data, espeically the vertex normals, and well, put it simply, makes reflections on those dense meshes look bad.
This thread is not about whether the meshes could be optimized. The whole thread is under the condition that the mesh data (and its density) CANNOT be reduced!

Anyway, @RyanB already stated the problem in this thread: https://forums.unrealengine.com/showthread.php?98518-Reflection-Banding-on-Concave-Metallic-Surfaces that the normal data gets packed into a 4-component vector of uint8 size which hurts accuracy

So, I of course went forward and tried an initial approach:

  1. replacing the FPackedNormal types with FVector4 (because the packed normal consists of 4 components, too) for static and dynamic meshes. I limited it to mesh-related instances.
  2. replaced the VET_PackedNormal EVertexElementType enum with VET_FLOAT4, i guess this is for the actual render ressources and needed?
  3. I went forth and fixed all the compile errors that arose from my changes, mainly typo stuff for example: TangentX.Vector.W to TangetX.W.
  4. I bumped the static mesh version number because I ran into problems when it tried to cache old meshes (my understanding of a GUID is that I just change the last literal and the version number is “bumped”?)

My problem is, that my knowledge of the engine is still limited in terms of C++ and whenever I try to compile the engine, i trigger a breakpoint because of some serialization problems.

My questions are:

  1. Is this approach alright at all or should I use another data type instead of FVector4? Should I maybe just try to increase the FPackedNormal size to unit16 per vector component?
  2. I read some warnings about TArray::BulkSerialize() and i am not sure whether they apply for FVector4 or my case in general
  3. What do I have to do in order to get the serialization process for the new vertex normal/tangent size to work?
  4. Some general guidelines about this problem would be great :slight_smile:
  5. In case FVector4 is applicable, what kind of performance hits are to be expected? Can it be that bad?

Assertion failed: SerializedElementSize == 0 || SerializedElementSize == ElementSize …\Engine\Source\Runtime\Core\Public\Containers\Array.h] [Line: 1262]
Expected 64, Got: 36

CallStack:


>	
        UE4Editor-Engine.dll!TArray<FModelVertex,TAlignedHeapAllocator<0> >::BulkSerialize(FArchive & Ar, bool bForcePerElementSerialization) Line 1262	
 	UE4Editor-Engine.dll!UModel::Serialize(FArchive & Ar) Line 276	
 	UE4Editor-CoreUObject.dll!FLinkerLoad::Preload(UObject * Object) Line 3175	
 	UE4Editor-CoreUObject.dll!EndLoad() Line 1245	
 	UE4Editor-CoreUObject.dll!LoadPackageInternal(UPackage * InOuter, const wchar_t * InLongPackageName, unsigned int LoadFlags, FLinkerLoad * ImportLinker) Line 1042
 	UE4Editor-UnrealEd.dll!UEditorEngine::Map_Load(const wchar_t * Str, FOutputDevice & Ar) Line 2374	
 	UE4Editor-UnrealEd.dll!UEditorEngine::HandleMapCommand(const wchar_t * Str, FOutputDevice & Ar, UWorld * InWorld) Line 5897	
 	UE4Editor-UnrealEd.dll!UEditorEngine::Exec(UWorld * InWorld, const wchar_t * Stream, FOutputDevice & Ar) Line 5391	
 	UE4Editor-UnrealEd.dll!UUnrealEdEngine::Exec(UWorld * InWorld, const wchar_t * Stream, FOutputDevice & Ar) Line 743	
 	UE4Editor-UnrealEd.dll!FEditorFileUtils::LoadMap(const FString & InFilename, bool LoadAsTemplate, const bool bShowProgress) Line 2014	
 	UE4Editor-UnrealEd.dll!FEditorFileUtils::LoadDefaultMapAtStartup() Line 3215	
 	UE4Editor-UnrealEd.dll!FUnrealEdMisc::OnInit() Line 299
 	UE4Editor-UnrealEd.dll!EditorInit(IEngineLoop & EngineLoop) Line 86


Tangent basis is calculated from normal and tangent. So you need to increase vertex tangent accuracy too. For normals/tangents 16bit per channel is enough so using float is bit too much. Can’t help with engine c++ code much.

Another idea:
I have solved this same problem with my own engine by using dithering noise. So you could make detail normal map that contain small(±1/127) random variations. This and temporal AA should smooth out vertex normal banding issues but it also might give bit rougher look overall.

Well, from what I understand, the changes that I did will never work, because the editor will always try to create some static meshes from Engine Content or DDC which is saved with the PackedNormals and deserializing these won’t work?
Maybe it is better to expand the editor with a button to recompute the Tangents/Normals in higher precision? But then, I would also have to change the underlying data-type. Puh, i am quite confused and out of ideas here.

We hit similar problems for the GDC McLaren demo we made. Fortunately, we fixed them and they will make it into public release in 4.12.

Regarding perf - if you are going from 32 bits to 128 bits, you will certainly see an impact. Because of this, we choose to keep them at 32 Bits, but used a better compression scheme and pay a much smaller cost(sometimes this cost can be completely hidden by memory latency) in the shader to decompress.

Regarding the crash you are hitting - its hard for me to advice exactly without seeing the code. What I can say is we had to update the data pipeline pre-DDC for the reasons you pointed out. My recommendation would be to wait for 4.12 if you can. If not, I’m working on getting the code integrated from our GDC branch into GitHub live development and you can grab the relevant changes from there.

What compression scheme you decided to use? Is tangents also packed using same scheme? Where I can find that pull request?

Zabir, thank you very very much for your answer!
It is not pressuring and I will take a close look onto the master branch commits but if it’s in 4.12, this is really great news!!! In fact, I was really bothered by the packednormals artifacts for a long time and decided to give it a try now, but great to hear that it was already fixed and hopefully makes it into master/4.12 anytime soon! :slight_smile:

It’s in! :slight_smile:

Copying //UE4/Dev-Rendering to Dev-Main (//UE4/Dev-Main) (Source: //UE4/Dev-Rendering @ 2943238)
https://github.com/EpicGames/UnrealEngine/commit/5f4d6e257c56dda55d9cac92476fdcbde38d4277

Yep, hopefully 4.12 is not yet branched. :slight_smile: