GSOC 26: normal (bump) map support - #9067
Conversation
|
Like the previous PR, I have some performance concerns about this happening all the time so I'd like to understand what the impact is. This one in particular seems like it adds extra data to geometry draws, which I imagine would have a noticeable impact on heavier sketches. Like my suggestion in the previous PR, it could be that we want to actually exclude some of this functionality from the shader if it's not used (with a separate shader, either separate source code or the same code + different One last thing, I think maybe the visual test example here could be clearer. Is it possible to use something like a sphere with a bump map and some lighting to make it closer to what the intended effect is that the end user would want? |
5d03f60 to
3eafa4f
Compare
|
One more request: should we add an option for bump height too, following the pattern threejs uses? I think it's relatively common for bump maps to use as much range as they can, going from black to white, to get the best precision they can on normals, and then in software scale it down. I see on Wikipedia there's a Should we also make methods to be able to set these similar to |
3eafa4f to
9255970
Compare
|
update, switched normal mapping to baked per vertex tangents and got webgpu working too, plus added bump strength and a bumpTexture() method. screen space tangents were coming out faceted on low poly meshes (dFdx of the position is flat per triangle), so baked fixes that and keeps the surface smooth. its all behind the two shader variant so plain lit materials get none of the tangent or normal map code, and tangents are only computed/uploaded for meshes that actually have a normal map, so no bump map scenes pay nothing extra. on perf, baked vs screen space came out about equal, on my machine a heavy scene of 121 lit bump mapped spheres ran ~50.1 fps screen space vs ~49.5 baked, and it held up equal across a couple other machines too so no regression from going baked. also ported it to webgpu. no preprocessor in wgsl so the shaders are functions of a flag now, the maps variant pulls in the tangent + normal sampling and the plain one leaves it out. added the -bm bump strength from the mtl so intensity is tunable, and a bumpTexture() method so you can set a normal map in code like texture(). bumpTexture(null) clears it and it scopes with push/pop, and it builds tangents on the fly for shapes that dont have their own. both renderers render a lit bump mapped sphere and tests pass on both. stacked on #9066 so that one goes in first |
| * | ||
| * @method bumpTexture | ||
| * @param {p5.Image|p5.MediaElement|p5.Graphics|p5.Texture|p5.Framebuffer|p5.FramebufferTexture} tex normal map, or `null` to clear it. | ||
| * @param {Number} [scale] bump strength multiplier. Defaults to 1. |
There was a problem hiding this comment.
We can add the default value right into the jsdoc too:
| * @param {Number} [scale] bump strength multiplier. Defaults to 1. | |
| * @param {Number} [scale=1] bump strength multiplier. Defaults to 1. |
There was a problem hiding this comment.
done, added [scale=1]. i also renamed the method and rewrote this whole doc block, more on that in the thread just below
| this, | ||
| materialVertexShader, | ||
| materialFragmentShader, | ||
| materialVertexShader({ useTextureMaps }), |
There was a problem hiding this comment.
thanks! the reflection setup made it fall into place once the wgsl declared the tangent attribute + normal texture
| * @param {Number} [scale] bump strength multiplier. Defaults to 1. | ||
| * @chainable | ||
| */ | ||
| fn.bumpTexture = function (tex, scale) { |
There was a problem hiding this comment.
Similar to this, should we make methods for the shininess texture and the other texture properties?
There was a problem hiding this comment.
good call, added specularTexture(), ambientTexture() and shininessTexture() too. they mirror this one, set the map and turn on the term it modulates, and null clears it. added tests for all four in p5.RendererGL.js
| * Sets a normal (bump) map to add surface detail to shapes under lighting. | ||
| * | ||
| * `bumpTexture()` works like <a href="#/p5/texture">texture()</a>, but for a | ||
| * tangent-space normal map. Call it before drawing a shape and its surface |
There was a problem hiding this comment.
We should probably mention that we read the brightness of the texture to determine bump height, and call it a bump map rather than a normal map, since those are different.
Would be good to add an example here too.
There was a problem hiding this comment.
so this one is actually a normal map, not a bump map, we decode the rgb as the tangent space normal (rgb*2-1), we dont read brightness. since bump and normal maps are different like you said, i renamed the method to normalTexture() to be honest about what it is (and its the same kind gltf uses). rewrote the doc to say that and added an example.
|
One last thought: it looks like the gltf format supports normal maps instead of bump maps. I'm sort of on the fence about what to do about that, because bump maps feel much easier to create generatively and are conceptually a lot easier to understand. But to fully support a gltf import, we'd want access to normal maps. We could have both available in the future, but also they can't both be active at once -- if we add something like |
|
on the gltf / bump vs normal question, one clarification first: our current implementation is already a normal map, we decode the rgb as the tangent space normal (rgb*2-1) and dont read brightness, so its the gltf compatible path. i renamed the public method to normalTexture() in this pr to reflect that (and it lines up with the internal _normalTex / normalTexture partState we already had). on future proofing, yeah i think one texture slot + an interpretation flag is the clean way, exactly like you suggested. a future bumpTexture() (the brightness -> gradient kind, which is the easier to author one you mentioned) would set the same _normalTex but flip a mode flag, and the shader would branch on it. only one active at a time, and adding it later is purely additive, so we shouldnt have to change normalTexture() or any current api. happy to take that on as a follow up once this lands. |
what
adds normal (bump) map support (
map_Bump), the last of the mtl texture maps. a normal-mapped material perturbs the surface normal per pixel so lighting shows surface detail that isn't in the geometry.how
normal mapping needs a per-vertex tangent basis, which p5 didn't have. the pieces:
p5.Geometry.computeTangents()(new) computes a per-vertex surface tangent from the uvs (edge vectors + uv deltas, accumulated per vertex, Gram-Schmidt orthogonalised against the normal). the bitangent handedness is stored in the tangent'swso the shader can rebuild it ascross(normal, tangent) * w.buildMaterialPartshands each part its slice, mirroring how normals flow.aTangentvertex attribute (defaults to a dummy so it's always valid; the shader only uses it when a normal map is bound).phong.verttransforms the tangent into view space alongside the normal and passes it on.phong.fragrebuilds the TBN basis, samples the normal map, and perturbs the normal, gated byuHasNormalMapso untextured draws are unaffected.zero regression
the
aTangentattribute and shader branch are gated/defaulted so existing 3D draws are untouched. confirmed by the full visual + unit suite staying green.testing
computeTangents()unit test with a known triangle (asserts the exact tangent vector + handedness), plus the no-uvs caseloadModeltest: a normal-mapped obj computes tangents, and the part carries both the normal map and its own per-vertex tangentsmtlToPartStateunit test for the normal map fieldpart of the gsoc multi-material .mtl work, follows #8879, #8955, #9063, and #9066.