testrender: Fix uv derivatives for testrender [#1978] #2037
Conversation
src/testrender/simpleraytracer.cpp
Outdated
| Dual2<Vec3> P = r.point(t); | ||
| // We are missing the projection onto the surface here | ||
| sg.P = P.val(); | ||
| sg.N = scene.normal(P, sg.Ng, id, u, v); | ||
| // Projecting onto the surface here | ||
| scene.project(P, sg.N, sg.I); | ||
| sg.dPdx = P.dx(); | ||
| sg.dPdy = P.dy(); | ||
| sg.N = scene.normal(P, sg.Ng, id, u, v); | ||
| Dual2<Vec2> uv = scene.uv(P, sg.N, sg.dPdu, sg.dPdv, id, u, v); |
There was a problem hiding this comment.
I'm thinking of
Switching N also to Dual2 to track changing differentials due to curvature.
And maybe replacing r.point(), scene.normal(), scene.uv() with one unified function so I can reuse some intermediate calculations.
There was a problem hiding this comment.
Changing the ShaderGlobals is a Big Deal, since it alters the binary interface between the renderer and the shading system, and changes memory layout that the JITed code needs to be aware of. We should do that only with some discussion among the group, and it can't be backported to release branches because it's a breaking change.
There was a problem hiding this comment.
Yeah that can be done as a second step (assuming people agree). Computing dNdx and dNdy for triangle meshes with interpolated normals is not that simple unfortunately (especially if you want want the values to change smoothly across the mesh).
We originally had N specified with derivatives a long time ago but removed it after we realized its tricky to compute a good approximation for those derivatives. But if there is interest, we could potentially look into this again.
There was a problem hiding this comment.
Oh, I wasn't clear. I didn't mean changing the SG interface. Only local update to testrender.
Should it be enough to update ray.spread after bsdf sampled, but before tracing this ray? Theoretically, we should know raytype, and dNdx, dNdx of the origin point, but ray.spread is 1D... and it's not propagated but assigned from roughness... and raytype is hardcoded as Ray::DIFFUSE for everything...
The idea of having accurate derivatives after reflection and refraction from the paper is so tempting. But I also understand that testrender is just an example, and all that is too much.
Anyway, happy to discuss and hear your thoughts on it.
There was a problem hiding this comment.
For testrender only - we could definitely try it out. In any case, your current change looks like a good improvement on its own, we can fine tune it in a second pass.
|
Aside from those minor notes above, the logic seems like its correct. Results also look promising. Someone with access to a working implementation might want to spot check it too for how to handle the grazing angle cases and other situations that can divide by 0. A follow up might be to add a version that guarantees continuity of the derivatives between triangles - but that requires extra storage and is fine to leave as a second step. |
|
I think the only remaining step would be to update the reference images for the failing tests. |
Signed-off-by: Alexey Smolenchuk <[email protected]>
Signed-off-by: Alexey Smolenchuk <[email protected]>
Signed-off-by: Alexey Smolenchuk <[email protected]>
Signed-off-by: Alexey Smolenchuk <[email protected]>
AlexeySmolenchuk
force-pushed
the
fix_uv_derivatives_testrender
branch
from
dda4090 to
629fd9f
Compare
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
|
I took the liberty of quickly updating the ref image and formatting and pushed those to your branch. Let's see if those pass all the tests now. |
|
I think we're almost there! The only one failing is the optix test. I think some of the changes you made to simpleraytracer need to also be applied to optixrantracer? |
|
I have good news, @AlexeySmolenchuk! The OptiX test failures we're seeing here are also happening in main and other branches, too, so they appear unrelated to this PR. So I am going to merge what you have here, and we'll figure out how to fix the OptiX support separately. Since I never merged anything that didn't pass all tests including OptiX, I suspect that it's not a problem we introduced at all, but rather something that changed on the GH runners that we need to adapt to. |
lgritz
changed the title
lgritz
merged commit 6c91fb8
into
AcademySoftwareFoundation:dev-1.14
lgritz
added
bug
|
I managed to make a very hacky OptiX build, and I can say that dPdx and dPdy work as expected. Derivatives of u and v also match the CPU version when visualised using emission(). What's not working is accessing the proper mip level from the texture call. Even if I read it like Reading derivatives of a texture signal is also incorrect. |
|
Thanks, and sorry to distract you with this optix issue. The widespread failure of the optix jobs has been conclusively tied to an update of the docker container that occurred last week. |
3 participants
Description
Fix dudx, dudy, dvdx, dvdy, as well as dpdx, dpdy in testrender. Now, bump mapping from both UV-based mapping and 3D-based patterns works well. Mip-Map selection also works fine now. Images in Issue: #1978
This PR is intended to validate math and discuss tests.
Fixes #1978
Tests
This fix could potentially affect many other tests since dpdx, dpdy are changed.
I'm going to add tests representing bump and mipmapping in this PR.