PBR Materials - #358
Merged
Merged
Conversation
Materials used to be just a base color and a normal map, with the roughness and metalness coming from a global config, so the shading couldn't be physically based at all. Material model: - `Material` carries metallic-roughness and emissive, as factors and textures, cooked from glTF including `KHR_materials_emissive_strength` - internally a material is a diffuse albedo, a specular reflectance at normal incidence, and a roughness. The metallic-roughness inputs are converted in `material_from_metallic_roughness` where the textures are sampled: folding it into the factors instead would only be exact for the materials without a metallic-roughness texture, and doing it fully at cook time would mean merging the base color and metalness images into per-material diffuse and specular maps, giving up the sharing of the texture assets. - `brdf.inc.wgsl` has the shared BRDF: GGX distribution, height correlated Smith visibility, Schlick Fresnel. Lobe sampling is shared in `sampling.inc.wgsl`, environment sampling in `env-light.inc.wgsl`, and the geometry with the material fetch in `hit.inc.wgsl`. - ray tracing: material G-buffer, diffuse and specular lighting kept apart, candidates drawn from the BRDF as well as from the environment with MIS between them, since a narrow specular lobe can't be resolved by sampling the light alone - `ProceduralGeometry` gained the PBR factors, and builds an acceleration structure, so it can be ray traced at all - `RasterConfig` no longer overrides the material properties, and the cooked model format changed, so asset caches need to be cleared Canonical renderer: - `RayTracer::path_trace` traces full paths with BSDF sampling and next event estimation on the environment, combined by MIS, accumulated over the frames with no reuse and no denoising. Configured by `PathTraceConfig`, reset by `FrameConfig::reset_accumulation` or by moving the camera. Available in the scene example and in the engine. Color space: - the rasterizer was encoding gamma in the shader, which double corrected on a surface configured with the default `ColorSpace::Linear`. Both render paths now produce linear values, leaving the encoding to the surface, and an XR swapchain picks its format accordingly. Tests: - `parse_shaders` validates the renderer shaders too, which caught `fill-gbuf.wgsl` missing the `wgpu_binding_array` enable directive - new snapshots of a metallic-roughness sphere grid through the rasterizer, through ReSTIR, and through the canonical renderer, with the real-time result cross-checked against the ground truth - `gltf_material_test` covers the cooking of the PBR factors Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AwMzwvzypi5eqZLxgjUQX1
Three follow-ups from the review. The rasterizer encoding gamma in the shader wasn't wrong, it was the only thing correct for where it runs. `xr_recommended_surface_config` asks for `ColorSpace::Linear`, but `select_xr_swapchain_format` answers it with a plain format, and an XR runtime passes those straight to the compositor instead of linearizing them. So the XR swapchain needs encoded values, which is exactly what the raster path produced, while a window surface under the same request gets an sRGB format and needs linear ones. Nothing told the renderers which of the two they were writing to. So `SurfaceInfo` now reports the color space of the surface contents, meaning what we have to produce: `Linear` when an sRGB format or a linear display space does the encoding for us, `Srgb` when the values are passed through. Both render paths encode accordingly, with the standard sRGB transfer function rather than a 2.2 power, so the two kinds of surface look the same. The XR format choice is left alone, and the raster output on a plain surface matches the previous reference at SSIM 0.9988. "Metallic" is what the glTF spec calls the field, but it reads as an adjective where we want a quantity, so the value is `metalness` now. The name "metallic-roughness" stays where it refers to the glTF texture or the model itself. Canonical is no longer a separate call with a config of its own: - `RenderMode` picks what the ray tracer does, and `RayTracer::render` performs the frame, so a user configures a mode instead of calling a different method. `ray_trace` and `denoise` became internal steps. - `RayConfig` describes the sampling for both of the modes: `num_environment_samples` and `num_brdf_samples` are the light and material samples taken at a shading point, `max_bounces` limits the path length, and `max_accumulated_samples` caps the accumulation. - ReSTIR draws those two counts of candidates and weights them by the mixture of their densities, which is multi-sample MIS with counts, and replaces the heuristic that used to pick a strategy at random. The canonical mode continues a path along each of its material samples and takes the light samples at every vertex. The explicit counts also brought the real-time result closer to the ground truth: the cross-check against the canonical snapshot went from 4.33 to 3.94 out of 255. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AwMzwvzypi5eqZLxgjUQX1
Reverts the `SurfaceInfo::color_space` I added: an application states the color space it works in when configuring the surface, so reporting it back is redundant. `blade-render` takes it as `RenderConfig::color_space`, next to the surface info the application already passes in. That leaves the XR path, which was the reason the information seemed to be missing: `xr_recommended_surface_config` asked for `ColorSpace::Linear`, while `select_xr_swapchain_format` answered it with a plain format that the runtime passes to the compositor as-is. The request wasn't honored, so no one could rely on it. An XR swapchain has no way to declare a color space, so its format is what honors the request: an sRGB one for `Linear`, which the runtime converts, and a plain one for `Srgb`, which it passes through. The recommended configuration now asks for `Srgb`, matching the plain format that it ends up with, which is the same format as before this change - the swapchain the asteroids example gets is unchanged, and so is the encoding it writes. `xr_recommended_surface_config` and `create_xr_surface_configured` became public, so that an application can hold the configuration of its XR surface rather than having it hidden inside `create_xr_surface`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AwMzwvzypi5eqZLxgjUQX1
The GPU test job failed on both Lavapipe hosts while passing on macOS, in the two heaviest tests: the accumulated ReSTIR grid and the canonical path trace. Neither was wrong, both were killed at the five second readback timeout, with no validation error and no device loss. Two things were against them. Software rasterizing dozens of accumulated ray traced frames is slow to begin with — the binary takes 18s on CI against 1.6s on Lavapipe locally. On top of that the suite runs ten tests at once, each standing up its own GPU context, and the ray traced ones also building acceleration structures and full sized render targets, so on a shared runner they compete for the same few cores and the heaviest ones stretch by much more than their own cost. So: run one test at a time, as the GLES step already did, and size the timeout for the slowest thing that legitimately runs. The timeout is there to stop a wedged GPU from hanging the suite, not to assert a frame time, so it should not be set from what a real device manages. Serializing costs nothing — the work is CPU bound either way, and the whole suite still finishes in 5s on Lavapipe locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019vTxtSJSvMtFaZJNeWZVAG
A consumer outside the renderer — a neural upscaler, a capture tool, an analysis pass — can currently only see the tone mapped result, which has already thrown away both the dynamic range and everything the renderer knew about the geometry behind each pixel. - `PostProcConfig::tone_map` can be cleared to leave the composed radiance alone. The exposure controls are unused when it is off, and the display transfer function is skipped along with the curve, since it is only defined over the display range. Rendering into a floating point target is what makes this observable; a fixed point one still clamps. Breaking: the struct gained a field. - `RayTracer::view_gbuffer` returns the depth, tangent basis, flat normal, diffuse albedo, specular reflectance, emissive, and motion of the frame that was last prepared, with each format and encoding documented so a consumer does not have to read the shaders. The views belong to the renderer and stay valid until the next `resize_screen`. Tests: - `hdr_capture_is_unclipped` checks the capture exceeds the display range, and that tone mapping it on the CPU reproduces the display capture, which pins it to the same signal one stage earlier rather than to some other buffer that happens to be bright - `gbuffer_views_describe_the_rendered_frame` binds the views from a compute shader and checks the roughness spans the range the material grid was built with, so a buffer read from the wrong place or before the scene was drawn would not pass, and that the basis quaternion decodes to the geometric normal as documented - `render_ray_traced_grid_as` gained an `inspect` hook, so a test can reach the renderer's own state without duplicating the setup - `OffscreenTarget` reads back any texel size, not only 8-bit Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019vTxtSJSvMtFaZJNeWZVAG
The canonical renderer combines two ways of finding the light at a vertex: next event estimation, which samples the environment directly, and a BSDF sample that may run into it on the way out. The balance heuristic splits the contribution between them, so that neither is counted twice. At the last vertex of a path there is no second strategy. The loop breaks after the next event estimation, and the BSDF sample it was weighted against is never taken, so the share held back for it was simply lost. The weight is now the whole contribution whenever the path is about to end, which is what `will_extend` decides, and it also says what the loop was already testing for in its break condition. How much this cost depends on how much throughput the path had left at that last vertex, so deep paths hid it well: the material grid at three bounces moves by SSIM 0.9998, and the cross-check against ReSTIR is unchanged at 3.96 out of 255. At `max_bounces` of zero, though, the only vertex there is was losing a share of everything, and zero bounces is exactly the configuration that means direct lighting and nothing else. A white furnace shows it without a reference to compare against: a lone convex sphere cannot bounce light onto itself, so a direct-only render of one has to match a converged one, and an energy conserving BRDF has to leave it indistinguishable from the environment behind it. It was rendering visibly darker instead. It now matches the converged render exactly, to the mean code value over the sphere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaDekFuFvymYai37WQ7SDr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.