perf(ui): gate per-pixel clip compares behind a flags bit (#50) #91
6 changed files with 192 additions and 9 deletions
perf(ui): gate per-pixel clip compares behind a flags bit (#50)
uiResolveScreenPixel ran four float compares against hdr.clipRectPx for
every pixel of every UI dispatch, even though the clip rect is the
full-surface default {0,0,1e9,1e9} in the overwhelmingly common case.
Reserve the high bit of the header flags word (UI_FLAG_CLIP) and have
FillHeader set it only when the clip rect is narrower than the surface
(extracted into the unit-testable UIRenderer::ClipFlags). The shader
gates the compares on the bit; the branch is push-constant-uniform so it
never diverges. When the rect covers the surface the skipped compares
could never reject an in-surface pixel, so output is pixel-identical.
Mirrored on the WebGPU/WGSL path (dom-webgpu.js) for parity. Adds the
UIClipFlag CPU test covering the gate decision.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
commit
65c19ea84a
|
|
@ -256,12 +256,17 @@ struct UIDispatchHeader {
|
|||
@group(1) @binding(0) var outTex : texture_storage_2d<rgba8unorm, write>;
|
||||
@group(1) @binding(1) var prevTex : texture_2d<f32>;
|
||||
|
||||
// Renderer-reserved high flag bit (mirrors ui-shared.glsl::UI_FLAG_CLIP).
|
||||
const UI_FLAG_CLIP : u32 = 0x80000000u;
|
||||
|
||||
fn uiResolvePixel(coord: vec2<u32>) -> bool {
|
||||
if (coord.x >= hdr.surfaceW || coord.y >= hdr.surfaceH) { return false; }
|
||||
let fx = f32(coord.x); let fy = f32(coord.y);
|
||||
if (fx < hdr.clipX || fy < hdr.clipY) { return false; }
|
||||
if (fx >= hdr.clipX + hdr.clipW) { return false; }
|
||||
if (fy >= hdr.clipY + hdr.clipH) { return false; }
|
||||
if ((hdr.flags & UI_FLAG_CLIP) != 0u) {
|
||||
let fx = f32(coord.x); let fy = f32(coord.y);
|
||||
if (fx < hdr.clipX || fy < hdr.clipY) { return false; }
|
||||
if (fx >= hdr.clipX + hdr.clipW) { return false; }
|
||||
if (fy >= hdr.clipY + hdr.clipH) { return false; }
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -38,11 +38,27 @@ UIDispatchHeader UIRenderer::FillHeader(std::uint32_t itemBufferSlot,
|
|||
#else
|
||||
h.frameIdx = 0;
|
||||
#endif
|
||||
h.flags = flags;
|
||||
h.flags = ClipFlags(clipRectPx, window_->width, window_->height, flags);
|
||||
h._pad = 0;
|
||||
return h;
|
||||
}
|
||||
|
||||
std::uint32_t UIRenderer::ClipFlags(std::array<float,4> clipRectPx,
|
||||
std::uint32_t surfaceWidth,
|
||||
std::uint32_t surfaceHeight,
|
||||
std::uint32_t flags) noexcept {
|
||||
// The reserved kUIFlagClip bit signals "clip rect is narrower than the
|
||||
// surface". When the rect already covers the whole surface (the common
|
||||
// {0,0,1e9,1e9} default), the four per-pixel clip compares in
|
||||
// uiResolveScreenPixel can never reject an in-surface pixel, so we leave
|
||||
// the bit clear and the shader skips them — pixel-identical output.
|
||||
const bool clipCoversSurface =
|
||||
clipRectPx[0] <= 0.0f && clipRectPx[1] <= 0.0f
|
||||
&& clipRectPx[0] + clipRectPx[2] >= static_cast<float>(surfaceWidth)
|
||||
&& clipRectPx[1] + clipRectPx[3] >= static_cast<float>(surfaceHeight);
|
||||
return clipCoversSurface ? (flags & ~kUIFlagClip) : (flags | kUIFlagClip);
|
||||
}
|
||||
|
||||
std::uint32_t UIRenderer::ShapeText(Font& font, float pxSize,
|
||||
float x, float baselineY,
|
||||
std::string_view utf8,
|
||||
|
|
|
|||
|
|
@ -59,6 +59,12 @@ export namespace Crafter {
|
|||
};
|
||||
static_assert(sizeof(UIDispatchHeader) == 48);
|
||||
|
||||
// Reserved `flags` bit (mirrors shaders/ui-shared.glsl::UI_FLAG_CLIP).
|
||||
// FillHeader sets it when the clip rect is narrower than the surface so
|
||||
// the standard shaders can skip the per-pixel clip compares otherwise.
|
||||
// The remaining low bits stay free for user-defined feature flags.
|
||||
inline constexpr std::uint32_t kUIFlagClip = 0x80000000u;
|
||||
|
||||
// ─── standard item PODs (match GLSL std430) ─────────────────────────
|
||||
struct QuadItem {
|
||||
float x, y, w, h;
|
||||
|
|
@ -165,6 +171,18 @@ export namespace Crafter {
|
|||
std::array<float,4> clipRectPx = {0.0f, 0.0f, 1e9f, 1e9f},
|
||||
std::uint32_t flags = 0) const noexcept;
|
||||
|
||||
// Builds the header `flags` word: the caller's user flags with the
|
||||
// reserved kUIFlagClip bit set iff `clipRectPx` does not already cover
|
||||
// the whole surface. When the clip rect spans the surface, the four
|
||||
// per-pixel clip compares in uiResolveScreenPixel can never reject an
|
||||
// in-surface pixel, so the bit stays clear and the shaders skip them.
|
||||
// Static + dimension-parameterised so the decision is unit-testable
|
||||
// without a live Window; FillHeader feeds it the current window size.
|
||||
static std::uint32_t ClipFlags(std::array<float,4> clipRectPx,
|
||||
std::uint32_t surfaceWidth,
|
||||
std::uint32_t surfaceHeight,
|
||||
std::uint32_t flags) noexcept;
|
||||
|
||||
void DispatchQuads(GraphicsCommandBuffer cmd, std::uint32_t bufferSlot, std::uint32_t itemCount,
|
||||
std::array<float,4> clipRectPx = {0.0f, 0.0f, 1e9f, 1e9f});
|
||||
void DispatchCircles(GraphicsCommandBuffer cmd, std::uint32_t bufferSlot, std::uint32_t itemCount,
|
||||
|
|
|
|||
28
project.cpp
28
project.cpp
|
|
@ -413,6 +413,34 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
|||
memImpls.emplace_back("tests/MemoryTypeFallback/main");
|
||||
mc.GetInterfacesAndImplementations(ifaces, memImpls);
|
||||
cfg.tests.push_back(std::move(memTest));
|
||||
|
||||
// Issue #50: uiResolveScreenPixel now gates its per-pixel clip-rect
|
||||
// compares on the reserved kUIFlagClip bit, which FillHeader sets only
|
||||
// when the clip rect is narrower than the surface. The decision lives
|
||||
// in UIRenderer::ClipFlags — pure CPU logic over the clip rect and the
|
||||
// surface size — so this test drives it directly with synthetic
|
||||
// dimensions, no Window or GPU device needed at runtime.
|
||||
Test clipTest;
|
||||
Configuration& clc = clipTest.config;
|
||||
clc.path = cfg.path;
|
||||
clc.name = "UIClipFlag";
|
||||
clc.outputName = "UIClipFlag";
|
||||
clc.type = ConfigurationType::Executable;
|
||||
clc.target = cfg.target;
|
||||
clc.march = cfg.march;
|
||||
clc.mtune = cfg.mtune;
|
||||
clc.debug = cfg.debug;
|
||||
clc.sysroot = cfg.sysroot;
|
||||
clc.dependencies = cfg.dependencies;
|
||||
clc.externalDependencies = cfg.externalDependencies;
|
||||
clc.compileFlags = cfg.compileFlags;
|
||||
clc.linkFlags = cfg.linkFlags;
|
||||
clc.defines = cfg.defines;
|
||||
clc.cFiles = cfg.cFiles;
|
||||
std::vector<fs::path> clipImpls(impls.begin(), impls.end());
|
||||
clipImpls.emplace_back("tests/UIClipFlag/main");
|
||||
clc.GetInterfacesAndImplementations(ifaces, clipImpls);
|
||||
cfg.tests.push_back(std::move(clipTest));
|
||||
}
|
||||
|
||||
return cfg;
|
||||
|
|
|
|||
|
|
@ -27,10 +27,19 @@ struct UIDispatchHeader {
|
|||
vec4 clipRectPx; // (xy, wh) — every standard shader honors this
|
||||
uint itemCount;
|
||||
uint frameIdx;
|
||||
uint flags; // user-defined feature bits
|
||||
uint flags; // feature bits — high bit reserved by the renderer
|
||||
uint _pad; // reserved — keep zeroed
|
||||
};
|
||||
|
||||
// ─── header flag bits ───────────────────────────────────────────────────
|
||||
// `flags` is otherwise free for user-defined feature bits, but the renderer
|
||||
// reserves the high bit: UI_FLAG_CLIP is set by UIRenderer::FillHeader when
|
||||
// the clip rect does not already cover the whole surface. uiResolveScreenPixel
|
||||
// gates its four clipRectPx compares on this bit so the overwhelmingly common
|
||||
// full-surface case (clipRect == {0,0,1e9,1e9}) skips them entirely. The
|
||||
// branch is push-constant-uniform across the dispatch, so it never diverges.
|
||||
const uint UI_FLAG_CLIP = 0x80000000u;
|
||||
|
||||
// ─── standard item structs ──────────────────────────────────────────────
|
||||
// These match the C++ Crafter::QuadItem / CircleItem / ImageItem / GlyphItem
|
||||
// byte-for-byte under std430.
|
||||
|
|
@ -121,9 +130,11 @@ QuadItem LoadQuadItem(uint heap, uint i) {
|
|||
bool uiResolveScreenPixel(UIDispatchHeader hdr, out ivec2 screenPx) {
|
||||
uvec2 px = gl_GlobalInvocationID.xy;
|
||||
if (px.x >= hdr.surfaceSize.x || px.y >= hdr.surfaceSize.y) return false;
|
||||
if (float(px.x) < hdr.clipRectPx.x || float(px.y) < hdr.clipRectPx.y) return false;
|
||||
if (float(px.x) >= hdr.clipRectPx.x + hdr.clipRectPx.z) return false;
|
||||
if (float(px.y) >= hdr.clipRectPx.y + hdr.clipRectPx.w) return false;
|
||||
if ((hdr.flags & UI_FLAG_CLIP) != 0u) {
|
||||
if (float(px.x) < hdr.clipRectPx.x || float(px.y) < hdr.clipRectPx.y) return false;
|
||||
if (float(px.x) >= hdr.clipRectPx.x + hdr.clipRectPx.z) return false;
|
||||
if (float(px.y) >= hdr.clipRectPx.y + hdr.clipRectPx.w) return false;
|
||||
}
|
||||
screenPx = ivec2(px);
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
105
tests/UIClipFlag/main.cpp
Normal file
105
tests/UIClipFlag/main.cpp
Normal file
|
|
@ -0,0 +1,105 @@
|
|||
/*
|
||||
Crafter®.Graphics
|
||||
Copyright (C) 2026 Catcrafts®
|
||||
catcrafts.net
|
||||
|
||||
This library is free software; you can redistribute it and/or
|
||||
modify it under the terms of the GNU Lesser General Public
|
||||
License version 3.0 as published by the Free Software Foundation;
|
||||
|
||||
This library is distributed in the hope that it will be useful,
|
||||
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||
Lesser General Public License for more details.
|
||||
|
||||
You should have received a copy of the GNU Lesser General Public
|
||||
License along with this library; if not, write to the Free Software
|
||||
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
|
||||
*/
|
||||
|
||||
// Regression test for issue #50: uiResolveScreenPixel used to run four float
|
||||
// compares against hdr.clipRectPx for every pixel of every dispatch, even
|
||||
// though the clip rect is the full-surface default {0,0,1e9,1e9} in the
|
||||
// overwhelmingly common case. The compares are now gated on the reserved
|
||||
// kUIFlagClip bit, which UIRenderer::FillHeader sets (via ClipFlags) only when
|
||||
// the clip rect is actually narrower than the surface.
|
||||
//
|
||||
// ClipFlags is the pure CPU decision behind that gate: given a clip rect, the
|
||||
// surface size, and the caller's user flags, it returns the header flag word
|
||||
// with kUIFlagClip set iff the rect does not cover the whole surface. This
|
||||
// test drives it directly with synthetic dimensions — no Window or device.
|
||||
//
|
||||
// The contract it must hold for the shader gate to be correct:
|
||||
// * full-surface (and over-covering) clips must NOT set the bit, so the
|
||||
// shader skips the compares — and the skipped compares would never have
|
||||
// rejected an in-surface pixel anyway, so output is pixel-identical;
|
||||
// * any clip narrower than the surface on ANY edge MUST set the bit;
|
||||
// * the caller's user flag bits are preserved either way; only the reserved
|
||||
// high bit is touched.
|
||||
|
||||
#include <cstdlib>
|
||||
|
||||
import Crafter.Graphics;
|
||||
import std;
|
||||
using namespace Crafter;
|
||||
|
||||
namespace {
|
||||
|
||||
int failures = 0;
|
||||
|
||||
void Check(bool ok, std::string_view what) {
|
||||
std::println("{} {}", ok ? "PASS" : "FAIL", what);
|
||||
if (!ok) ++failures;
|
||||
}
|
||||
|
||||
constexpr std::array<float,4> kFullDefault = {0.0f, 0.0f, 1e9f, 1e9f};
|
||||
|
||||
bool ClipBitSet(std::array<float,4> rect, std::uint32_t w, std::uint32_t h,
|
||||
std::uint32_t userFlags = 0) {
|
||||
return (UIRenderer::ClipFlags(rect, w, h, userFlags) & kUIFlagClip) != 0u;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
int main() {
|
||||
constexpr std::uint32_t W = 1920, H = 1080;
|
||||
|
||||
// --- full-surface clips leave the bit clear (the fast path) -----------
|
||||
Check(!ClipBitSet(kFullDefault, W, H),
|
||||
"the {0,0,1e9,1e9} default does not set the clip bit");
|
||||
Check(!ClipBitSet({0.0f, 0.0f, float(W), float(H)}, W, H),
|
||||
"a clip exactly matching the surface does not set the clip bit");
|
||||
Check(!ClipBitSet({-5.0f, -5.0f, float(W) + 10.0f, float(H) + 10.0f}, W, H),
|
||||
"a clip that over-covers the surface does not set the clip bit");
|
||||
|
||||
// --- any narrower edge sets the bit -----------------------------------
|
||||
Check(ClipBitSet({0.0f, 0.0f, float(W) - 1.0f, float(H)}, W, H),
|
||||
"a clip one pixel short on the right edge sets the clip bit");
|
||||
Check(ClipBitSet({0.0f, 0.0f, float(W), float(H) - 1.0f}, W, H),
|
||||
"a clip one pixel short on the bottom edge sets the clip bit");
|
||||
Check(ClipBitSet({1.0f, 0.0f, float(W), float(H)}, W, H),
|
||||
"a clip inset on the left edge sets the clip bit");
|
||||
Check(ClipBitSet({0.0f, 1.0f, float(W), float(H)}, W, H),
|
||||
"a clip inset on the top edge sets the clip bit");
|
||||
Check(ClipBitSet({100.0f, 100.0f, 200.0f, 200.0f}, W, H),
|
||||
"a small interior clip rect sets the clip bit");
|
||||
|
||||
// --- user flag bits survive the decision ------------------------------
|
||||
constexpr std::uint32_t kUser = 0x0000000Fu; // four low user bits
|
||||
Check(UIRenderer::ClipFlags(kFullDefault, W, H, kUser) == kUser,
|
||||
"full-surface clip preserves user flags and clears the reserved bit");
|
||||
Check(UIRenderer::ClipFlags({0.0f, 0.0f, 200.0f, 200.0f}, W, H, kUser)
|
||||
== (kUser | kUIFlagClip),
|
||||
"narrow clip preserves user flags and sets the reserved bit");
|
||||
// A caller that (wrongly) pre-set the reserved bit must not flip the
|
||||
// decision for a full-surface clip — ClipFlags owns that bit.
|
||||
Check(UIRenderer::ClipFlags(kFullDefault, W, H, kUIFlagClip | kUser) == kUser,
|
||||
"full-surface clip clears a stale reserved bit the caller passed in");
|
||||
|
||||
if (failures != 0) {
|
||||
std::println(std::cerr, "{} case(s) failed", failures);
|
||||
return 1;
|
||||
}
|
||||
std::println(std::cout, "all UI clip-flag cases passed");
|
||||
return 0;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue