Tighten screenshot comparison coverage#1730
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the Playground visual regression configuration to better handle pixel-diff variability and to add new texture repetition baseline tests.
Changes:
- Added per-test pixel comparison tuning via
thresholdand/orerrorRatiofor several existing tests. - Added two new “Texture Repetition” playground entries with reference images and engine exclusions.
5b1c447 to
b85623e
Compare
bkaradzic-microsoft
left a comment
There was a problem hiding this comment.
Thanks for tightening the screenshot tolerances — the per-test threshold/errorRatio approach is the right direction. I ran the changed/added tests locally against a Playground build (D3D11 + OpenGL) and hit blocking issues on the re-enabled and new tests. Two are structural and independent of Babylon version; a few others I'd like you to confirm pass in your CI, since my local build may differ from your base.
Blocking:
- New Texture Repetition reference images are 24-bit RGB, but every existing reference is 32-bit RGBA. The harness
compare()requires equal decoded-buffer lengths, so both new tests throw an uncaughtReference data length (720000) must match render data length (960000)and exit 1 — they can never pass on Babylon Native. Please re-export both PNGs as RGBA (ideally captured from Native's own output rather than the web playground). - Un-excluding OpenGL for
GUI Near Menure-introduces a hard crash. On the OpenGL build it hitsBGFX FATAL: Failed to compile shader ... '=' : cannot convert from 'highp float' to 'bool'(renderer_gl.cpp:6956) — exactly the failure the reason you removed described. This aborts the entire OpenGL test run. The OpenGL exclusion should stay (e.g.excludedGraphicsApis: ["OpenGL"]) until the GUI fragment-shader codegen issue is fixed.
Please verify (may be environment-specific on my end):
- Re-enabled
GUI Gradient Radial/Linear with transparencydon't crash/hang here (good — that reason is stale), but they diff ~60k/~102k pixels, far aboveerrorRatio: 1.0(~1%).GUI Gradient LinearthrowsTypeError: A string was expected, andGUI Slatefails to evaluate withThe data size does not match width, height, and format. Can you confirm these actually pass in your run?
Minor: excludedEngines and dependsOn on the new entries aren't implemented by validation_native.js (only excludedGraphicsApis, onlyVisual, excludeFromAutomaticTesting are honored) — they'll be silently ignored, giving a false sense of gating.
|
Heads-up on CI: this branch is currently ~38 commits behind Could you please rebase onto |
Store the texture repetition references as 32-bit RGBA so their decoded buffers match Native screenshot output. Keep GUI Near Menu excluded from OpenGL until its bgfx shader compile failure is fixed, and remove browser-only metadata that validation_native.js does not implement.
b85623e to
4d42eb3
Compare
|
Rebased onto current |
Replace the deleted GUI Slate snippet with its readiness-aware successor. The replacement preserves the same HolographicSlate scene and is pixel-identical to the existing golden.\n\nUse a 0.75% error ratio based on three stable Metal/bgfx captures at 1,708 of 240,000 differing pixels with threshold 25. This remains substantially tighter than the default 2.5% allowance while accounting for backend edge-antialiasing drift. NativeWebGPU has zero pixels over threshold 25.\n\nThe replacement also exposed fractional dynamic texture dimensions in ThinNativeEngine; BabylonJS/Babylon.js#18709 contains the separate runtime fix needed for bgfx backends to reach screenshot comparison.
Use a synchronous HolographicSlate playground without a private readiness or render-loop workaround. The configured 60 validation frames produce the same image with and without the separate readiness-pump change, keeping these PRs independent.\n\nKeep the GUI gradient cases excluded on bgfx and document the concrete Canvas API gaps: gradient stroke styles, two-circle radial geometry, and endpoint-space linear mapping. These are rendering limitations rather than tolerances that should be hidden with a permissive error ratio.
Summary
Why
The default 2.5% screenshot budget permits roughly 6,000 differing pixels in a 600x400 image. That is enough to hide missing or materially underdrawn controls and compact material differences. The tighter per-test budgets preserve normal anti-aliasing/color tolerance while making those regressions actionable.
The three GUI gradient cases are not tolerance problems on bgfx. Forced Metal/bgfx runs show missing Canvas semantics for gradient stroke styles, two-circle radial geometry, and endpoint-space linear mapping, so they remain excluded rather than being marked passing with loose thresholds.
Dependencies
GUI Slate requires BabylonJS/Babylon.js#18709 to be present in BabylonNative before this PR merges. That fix coerces fractional native dynamic-texture dimensions before allocation.
This PR does not depend on #1762. The replacement Slate playground is synchronous and its configured 60 validation frames produce the same capture with and without the readiness pump.
Validation
git diff --check