-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
Seams with normal map and mirrored uvs #18565
Comments
I'm not sure but I think you hit here a design limitation. AFAICS, Babylon.js implements similar code in their BTW: It seems the code in |
Blender may also use the same type of calculation as it requires the same hack to make it work. It is maybe more understandable as asset size is maybe less of an issue and 32bit solves it. That is really weird though because you would think that the neutral vector is kind of the base line. Anyway, I'll continue to use my hack as it does the trick perfectly. Mirroring allows to use much smaller normal textures which means smaller loading time and better GPU performance. |
Can you share the model? Some issues like this can be avoided by computing vertex tangents and setting |
@donmccurdy I actually get |
@makc use MeshStandardMaterial and there are no errors: material = new THREE.MeshStandardMaterial({
metalness: 1,
roughness: 0,
normalMap: normalMap,
envMap: texture
}); I would've expected it to work for Phong, but not Basic or Lambert materials. We may need to update docs in that case. I'm also surprised you don't need to clone the material before setting |
@donmccurdy ok it does not break with MeshStandardMaterial, that is something. but the seam issue is still there even after computing tangents - https://jsfiddle.net/jz64ow0m/ |
Hm, ok — my earlier suggestion is in the right direction, but isn't going to work. On the bright side I think I've learned enough to explain the issue here. From PolyCount • Normal Map Technical Details:
In other words, my suggestion of computing tangents using BufferGeometryUtils is no good — the normal map needs to be created for those tangents, because the whole thing is relative to a tangent basis. I tried to reproduce your JSFiddle in Blender, by creating a plane, cutting it into two pieces, then UV unwrapping it with a very obvious seam: the two UV islands are rotated and separated from one another. I then baked the normal map in Blender, and you can see that even though it's a flat plane, the normal map has a different color for each UV island because of the different UV orientation: Unfortunately I've hit a bug while trying to export the tangents from that model: the seam is reduced, but still visible, and all reflections are rotated by 90º. I think that rotation is due to Blender's different +Z=up convention, and needs to be fixed in the glTF exporter. In short: normal maps crossing UV seams are hard, and I don't expect any change to |
A note about this line:
I'm only aware of two formats that store tangents: glTF and FBX. THREE.FBXLoader doesn't load tangents currently (@looeee I can try to create an example if we want to add this?), so GLTFLoader is the only option for now. |
And... here's one much, much simpler option if you want to avoid all that: use an object-space normal map instead of a tangent-space one. https://jsfiddle.net/donmccurdy/4zx1aumq/
|
@donmccurdy I dont know why you need to write the research paper above when topic starter already identified both the problem and the solution :)
No matter if you expect it to solve the problem or not, it does solve it - https://jsfiddle.net/cuotmdfL/
or do not use normal map it is like saying, just do not run into a bug. it works, butnot really helping it :) |
THREE.ShaderChunk.normal_fragment_maps = THREE.ShaderChunk.normal_fragment_maps
.split("texture2D( normalMap, vUv ).xyz * 2.0 - 1.0;")
.join("texture2D( normalMap, vUv ).xyz * 2.0 - 1.003921568627451;") I see your point, it fixes the specific issue reported here, but ... this patch is not a general solution to UV seams in normal maps. Please feel free to try the same patch on the model attached above. 😬 Do you feel it should be merged to |
it makes sense if it is established that all the normal maps out there use 128 for zero. if some round down to 127 instead, there again will be a problem, and I did not test enough normal maps on pixel level to be sure. |
It would be interesting to add this, although so far I haven't seen any requests for this feature. Since I've never used tangents it would take me some time to study how they work so that I could add them to the loader. Sounds like fun, but it's not something I have time for at the moment, unfortunately. In the meantime, I was able to create examples. Here's sphere and cube meshes in ASCII and binary FBX format with tangents and binormals (it's not possible to export just tangents, so maybe they always need to be included together?). |
@looeee ok, no problem! We had a couple models reported in glTF where UV seams and mirroring issues could be fixed with tangents, but I haven't seen a similar report for FBX, and understand if you don't want to spend time on it until/unless that happens. :) |
@donmccurdy > Please feel free to try the same patch on the model attached above and btw, it just so happens that the patch in question does, in fact, remove the seam on the model attached above - https://jsfiddle.net/742du8gv/ - under the condition that the normal map you saved there is replaced by single color 128,128,255 normal map, because yours is not entirely uniform. however, even with your normal map left as is the seam is significantly improved :P |
Just to follow up, here are fiddles based on #18565 (comment):
As you see, there are no seams at both 127 and 128 -based maps, but the expression becomes much less elegant. |
Are these calculated according to MikkTSpace? (sorry, slightly offtopic) |
At least BufferGeometryUtils.computeTangents() does not implement MikkTSpace. |
I'm not aware of any JavaScript MikktSpace implementation. There's no reason it couldn't be done, but it's a fair bit of work. And (in this particular case) it isn't the solution. |
Thank you fellas. I wasn't sure what the last word was on the tangent/threejs issue and it would have saved me one step in my 3D pipeline.. alas, so be it ;) |
With #18614, BTW: Is it okay to close this issue? AFAICS, the issue depends on how the model was authored, right? |
@Mugen87 TL;DR the issue was about neither 127 nor 128 sitting exactly in a middle between 0 and 255 - and then in certain situations that 0.5/255 difference is enough to create a seam.
that is correct, if not fixed you cant use the models auhored in this way - so no more
for you :) |
How does this fix look like? Is it the code from #18565 (comment)? |
Mugen, there were a couple of hacks discussed in the thread, but no PR so far. The simplest hack offered by @njarraud shifts normal map zero level to 128, thus causing to 0 and 255 to map incorrectly as a price. I posted fiddles for both this hack and 127-based hack I think. Then the more convoluted hack that maps all of (0,127,128,255) values "correctly" but not all the other values inbetween (and the expression is ugly). |
TBH I think what OP did (using onBeforeCompile to hack the shader) is the best solution. But unfortunately only few people would come up with it on their own. |
Thanks for the summary. In any event, it stays a hack and as long as other engines produce the same output as current |
This paragraph (and especially the forum post linked to on Polycount) seems to establish that the 128 is the norm for 'unsigned' normal maps: http://wiki.polycount.com/wiki/Normal_map#Flat_Color I'm not registered to look at Unreals code nor intellectually proficient to understand what Unity is doing here, but both generally offer bug-free normal map mirroring from an artists perspective. Not sure Blender is a good benchmark here since it's a 'it'll get done if somebody wants to do it' type of product (no front!) ;) edit: In substance designer/painter (likely number one texturing tools for 3d artists) 128/128/255 denotes 0 as well |
@cptSwing what does marmoset do with 0 and 255 values? if you place the zero at 128 and keep using simlpe linear function, you have two options: a) 255 mapping to something less than 1, or b) 0 mapping to something less than -1 (well, ok, you could hav both ends wrong, too - but I assume we would want to get aT least one right) |
Assuming mapN = ( map > 128 / 255 ) ? map * 255 / 127 - 128 / 127 : map * 255 / 128 - 1; Or, assuming mapN = ( map > 128 ) ? ( map - 128 ) / 127 : map / 128 - 1; |
Good question. I dug around a little but couldn't find anything that stuck out. I would assume the 16bit normal map helps though?? I found this blog post though, breaking down the two methods for unpacking normals and their pros/cons: http://www.aclockworkberry.com/normal-unpacking-quantization-errors/ |
@WestLangley -s hack would work, too. One could even do this and have a smooth derivative at 0. |
Description of the problem
I guess it may not be just a threejs issue but I ran into a problem when using a normal map with a mesh having mirrored uvs.
With the current code, my understanding is that the RGB value (128, 128, 255) which is supposed to represent a non perturbed normal by convention is read as (0.50196, 0.50196, 1.0) - in linear color space. When converted to [-1, 1] using the function
map * 2.0 - 1.0
the vector encoded in the texture becomes (0.00392, 0.00392, 1.0) when it shall be (0.0, 0.0, 1.0). This is the reason why when using a mesh with mirrored uvs and a normal texture, the two normals at a seam have different directions creating the artifact.It can be easily proven by modifying the perturbNormal2Arb function.
Replace
vec3 mapN = map * 2.0 - 1.0;
byvec3 mapN = map * 2.0 - 1.00392;
and the seams disappear. I attached two screenshots made using a fully metallic material and no roughness to make it very obvious:Not too sure how this problem is generally solved as this is not my area of expertise.
Three.js version
Browser
OS
The text was updated successfully, but these errors were encountered: