mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-12 10:33:20 +00:00
ifcviewer-web: render through an sRGB surface view (fix dark colors)
The fragment shader pre-decodes sRGB→linear to cancel the surface's automatic linear→sRGB write encoding, so the final bytes match the GL backend. That only holds when the render target is an sRGB format. On desktop the surface's preferred format already is (e.g. BGRA8UnormSrgb), but the browser canvas only offers plain BGRA8Unorm — so nothing re-encoded and the whole image (background + models) rendered ~3× too dark (authored bg 0.125,0.137,0.161 → ~32,35,41 collapsed to ~3,4,6). Fix: when the surface format isn't sRGB, render through an sRGB *view* of it — the standard WebGPU canvas pattern. surface_view_format_ is the sRGB sibling of surface_format_ (unchanged when already sRGB, so desktop is a no-op); configureSurface advertises it via viewFormats, the colour pipelines (main, MSAA target, edge) target it, and render() creates the surface view with it. The screenshot path still reads the base texture, so its BGRA byte-order check stays on surface_format_. Regression test: sample a 1x1 background pixel and assert it isn't crushed dark (R,B > 20). Verified visually too — bg is now the correct dark blue-gray and the cube is properly lit. 5/5 web smoke + 107/107 unit pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,9 +1,29 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { dirname, resolve } from 'node:path';
|
||||
import zlib from 'node:zlib';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
// Decode the first pixel (RGB) of a PNG buffer. For a 1x1 image the row-0
|
||||
// first pixel is filter-agnostic (every PNG predictor references zero
|
||||
// neighbours), so we can skip full filter handling.
|
||||
function firstPixelRGB(png) {
|
||||
let off = 8, colorType = 6;
|
||||
const idat = [];
|
||||
while (off + 8 <= png.length) {
|
||||
const len = png.readUInt32BE(off);
|
||||
const type = png.toString('ascii', off + 4, off + 8);
|
||||
const data = png.subarray(off + 8, off + 8 + len);
|
||||
if (type === 'IHDR') colorType = data[9];
|
||||
else if (type === 'IDAT') idat.push(data);
|
||||
else if (type === 'IEND') break;
|
||||
off += 12 + len;
|
||||
}
|
||||
const raw = zlib.inflateSync(Buffer.concat(idat));
|
||||
return [raw[1], raw[2], raw[3]]; // skip the row filter byte
|
||||
}
|
||||
|
||||
// End-to-end smoke test for the WebGPU build. Every bug from the bring-up
|
||||
// of camera input + file loading was exactly this shape — the page would
|
||||
// load but render blank, spam uncaptured WebGPU errors, or swallow mouse
|
||||
@@ -98,6 +118,28 @@ test('sample renders without any interaction (streaming settle loop)', async ({
|
||||
expect(gpuErrors, gpuErrors.join('\n')).toEqual([]);
|
||||
});
|
||||
|
||||
test('background renders in sRGB, not crushed-dark (surface sRGB view)', async ({ page }) => {
|
||||
// Regression for the dark-colors bug: the shader pre-decodes sRGB to cancel
|
||||
// the surface's linear→sRGB write encode, which only works on an sRGB
|
||||
// target. The browser canvas is plain Unorm, so without rendering to an
|
||||
// sRGB *view* the whole image renders ~linear (≈3× too dark): the authored
|
||||
// background (0.125,0.137,0.161 → ~32,35,41) collapses to ~3,4,6.
|
||||
await page.goto('/IfcViewerWeb.html');
|
||||
await page.waitForFunction(
|
||||
() => !!(window.Module && window.Module._app_ptr), null, { timeout: 30_000 });
|
||||
await page.waitForTimeout(1200);
|
||||
|
||||
const box = await page.locator('#viewer-canvas').boundingBox();
|
||||
// A 1x1 sample of a top-left corner — background, clear of the framed cube.
|
||||
const px = await page.screenshot({
|
||||
clip: { x: Math.round(box.x + 6), y: Math.round(box.y + 6), width: 1, height: 1 },
|
||||
});
|
||||
const [r, g, b] = firstPixelRGB(px);
|
||||
// Correct sRGB background is ~(32,35,41); the bug crushes it to <10.
|
||||
expect(r, `background too dark — sRGB encode missing (got ${r},${g},${b})`).toBeGreaterThan(20);
|
||||
expect(b).toBeGreaterThan(20);
|
||||
});
|
||||
|
||||
test('loads a user-picked sidecar through the Blob.slice byte-range path', async ({ page }) => {
|
||||
// Exercises #88: the picked File is read via Blob.slice (metadata head/tail
|
||||
// + per-chunk byte ranges) WITHOUT copying the whole file into the wasm
|
||||
|
||||
@@ -993,7 +993,7 @@ bool ViewportCore::buildPipelines() {
|
||||
|
||||
// ---- Render pipeline -------------------------------------------------
|
||||
WGPUColorTargetState color_target = {};
|
||||
color_target.format = surface_format_;
|
||||
color_target.format = surface_view_format_; // sRGB view (see configureSurface)
|
||||
color_target.writeMask = WGPUColorWriteMask_All;
|
||||
|
||||
WGPUFragmentState frag = {};
|
||||
@@ -1293,6 +1293,22 @@ void onUncapturedError(WGPUDevice const* /*device*/,
|
||||
void* /*ud1*/, void* /*ud2*/) {
|
||||
Log::warn() << "[wgpu device error " << int(type) << "] " << svToStr(message);
|
||||
}
|
||||
|
||||
// The fragment shader pre-decodes sRGB→linear to cancel the surface's
|
||||
// automatic linear→sRGB write encoding (so the final bytes match the GL
|
||||
// backend). That only works when the render target is an sRGB format. On
|
||||
// desktop the surface's preferred format already is (e.g. BGRA8UnormSrgb);
|
||||
// the browser canvas only offers the plain Unorm format, so we render to an
|
||||
// sRGB *view* of it instead (configured via viewFormats). Maps a Unorm
|
||||
// surface format to its sRGB sibling; returns the input unchanged when it is
|
||||
// already sRGB (or has no sibling), so the desktop path is untouched.
|
||||
WGPUTextureFormat srgbViewFormat(WGPUTextureFormat f) {
|
||||
switch (f) {
|
||||
case WGPUTextureFormat_BGRA8Unorm: return WGPUTextureFormat_BGRA8UnormSrgb;
|
||||
case WGPUTextureFormat_RGBA8Unorm: return WGPUTextureFormat_RGBA8UnormSrgb;
|
||||
default: return f;
|
||||
}
|
||||
}
|
||||
} // namespace
|
||||
|
||||
bool ViewportCore::createPool() {
|
||||
@@ -1500,10 +1516,12 @@ bool ViewportCore::initWgpu(bool web_limits) {
|
||||
Log::warn() << "wgpuSurfaceGetCapabilities returned no formats";
|
||||
return false;
|
||||
}
|
||||
surface_format_ = caps.formats[0];
|
||||
surface_format_ = caps.formats[0];
|
||||
surface_view_format_ = srgbViewFormat(surface_format_);
|
||||
wgpuSurfaceCapabilitiesFreeMembers(caps);
|
||||
|
||||
Log::info() << "wgpu init OK; surface format = " << int(surface_format_);
|
||||
Log::info() << "wgpu init OK; surface format = " << int(surface_format_)
|
||||
<< " view format = " << int(surface_view_format_);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -1592,11 +1610,13 @@ void ViewportCore::initWgpuAsyncWeb(std::function<void(bool)> on_complete) {
|
||||
delete c;
|
||||
return;
|
||||
}
|
||||
c->core->surface_format_ = caps.formats[0];
|
||||
c->core->surface_format_ = caps.formats[0];
|
||||
c->core->surface_view_format_ = srgbViewFormat(c->core->surface_format_);
|
||||
wgpuSurfaceCapabilitiesFreeMembers(caps);
|
||||
|
||||
Log::info() << "[web init] wgpu device + surface ready (format="
|
||||
<< int(c->core->surface_format_) << ")";
|
||||
<< int(c->core->surface_format_) << " view format="
|
||||
<< int(c->core->surface_view_format_) << ")";
|
||||
c->on_complete(true);
|
||||
delete c;
|
||||
};
|
||||
@@ -3979,7 +3999,7 @@ void ViewportCore::ensureMsaaColorTexture(int w, int h) {
|
||||
desc.size.width = std::uint32_t(w);
|
||||
desc.size.height = std::uint32_t(h);
|
||||
desc.size.depthOrArrayLayers = 1;
|
||||
desc.format = surface_format_;
|
||||
desc.format = surface_view_format_; // matches the surface sRGB view + main pipeline target
|
||||
desc.mipLevelCount = 1;
|
||||
desc.sampleCount = kViewportSampleCount;
|
||||
desc.label = svFromCStr("ifcviewer-wgpu.msaa_color");
|
||||
@@ -4095,7 +4115,7 @@ bool ViewportCore::buildEdgePipeline() {
|
||||
blend.alpha.operation = WGPUBlendOperation_Add;
|
||||
|
||||
WGPUColorTargetState target = {};
|
||||
target.format = surface_format_;
|
||||
target.format = surface_view_format_; // sRGB view, matches the main pass target
|
||||
target.blend = &blend;
|
||||
target.writeMask = WGPUColorWriteMask_All;
|
||||
|
||||
@@ -5081,6 +5101,13 @@ void ViewportCore::configureSurface(int width_px, int height_px) {
|
||||
WGPUSurfaceConfiguration cfg = {};
|
||||
cfg.device = device_;
|
||||
cfg.format = surface_format_;
|
||||
// When the surface's own format isn't sRGB (browser canvas), render to an
|
||||
// sRGB view of it so the shader's sRGB encode-cancel lands the same way it
|
||||
// does on desktop. Advertise the view format so the view is creatable.
|
||||
if (surface_view_format_ != surface_format_) {
|
||||
cfg.viewFormatCount = 1;
|
||||
cfg.viewFormats = &surface_view_format_;
|
||||
}
|
||||
// CopySrc lets the screenshot path copy the surface texture back to
|
||||
// host memory. Trivial cost on all known backends.
|
||||
cfg.usage = WGPUTextureUsage_RenderAttachment | WGPUTextureUsage_CopySrc;
|
||||
@@ -5403,7 +5430,19 @@ void ViewportCore::render() {
|
||||
return;
|
||||
}
|
||||
|
||||
WGPUTextureView view = wgpuTextureCreateView(surf_tex.texture, nullptr);
|
||||
// Render through an sRGB view of the surface texture. On desktop the
|
||||
// surface is already sRGB so the view format matches the texture (a plain
|
||||
// default view); on web the surface is plain Unorm and this view is its
|
||||
// sRGB sibling (advertised via cfg.viewFormats) so the shader's sRGB
|
||||
// encode-cancel lands correctly. The screenshot path still reads the base
|
||||
// texture, so its BGRA byte-order check stays on surface_format_.
|
||||
WGPUTextureViewDescriptor view_desc = {};
|
||||
view_desc.format = surface_view_format_;
|
||||
view_desc.dimension = WGPUTextureViewDimension_2D;
|
||||
view_desc.mipLevelCount = 1;
|
||||
view_desc.arrayLayerCount = 1;
|
||||
view_desc.aspect = WGPUTextureAspect_All;
|
||||
WGPUTextureView view = wgpuTextureCreateView(surf_tex.texture, &view_desc);
|
||||
|
||||
updateFrameUniforms();
|
||||
|
||||
|
||||
@@ -682,6 +682,10 @@ private:
|
||||
WGPUQueue queue_ = nullptr;
|
||||
WGPUSurface surface_ = nullptr;
|
||||
WGPUTextureFormat surface_format_ = WGPUTextureFormat_Undefined;
|
||||
// Format the colour pipelines + surface view actually render through. Equals
|
||||
// surface_format_ on desktop (already sRGB); on web it's the sRGB sibling of
|
||||
// the plain-Unorm canvas format so the shader's sRGB encode-cancel works.
|
||||
WGPUTextureFormat surface_view_format_ = WGPUTextureFormat_Undefined;
|
||||
bool surface_configured_ = false;
|
||||
|
||||
// ---- Pipelines + bind-group layouts (built once after init) -------------
|
||||
|
||||
Reference in New Issue
Block a user