CSG: phi-wedge support for ZSphere/Cylinder (bake startPhi/deltaPhi)#345
CSG: phi-wedge support for ZSphere/Cylinder (bake startPhi/deltaPhi)#345ggalgoczi wants to merge 3 commits into
Conversation
G4Sphere/G4Tubs with a phi wedge previously aborted in U4Solid. Bake the wedge into the primitive instead of composing CSG_INTERSECTION(ZSphere/Cylinder, PhiCut), which leaks rays whose origin is outside the wedge. sn::ZSphere/Cylinder gain 5-arg overloads parking startPhi/deltaPhi in the cx/cy slots; the csg_intersect_leaf_{zsphere,cylinder}.h leaves clip candidates to the wedge; U4Solid::init_Sphere_/init_Tubs emit the phi-baked primitives; CSGNode cylinder AABB no longer treats cx/cy as the centre. deltaPhi==0 (or >=2pi) => no clip, so non-phi geometry is unchanged.
…der)
Self-checking ctest for the per-primitive phi-wedge clip in
csg_intersect_leaf_{zsphere,cylinder}.h: asserts a ray whose near surface
candidate is outside the wedge still recovers the in-wedge hit (the original
CSG_INTERSECTION(prim,PhiCut) leak), and a ray crossing entirely outside the
wedge misses. Exit code is meaningful so regressions fail under ctest.
ggalgoczi
force-pushed
the
csg-phi-wedge-primitives
branch
from
5c6c76b to
8c2414b
Compare
Could you remind me where was this happening? |
There was a problem hiding this comment.
Pull request overview
This PR adds phi-wedge support for G4Sphere/G4Tubs by baking startPhi/deltaPhi directly into the CSG_ZSPHERE / CSG_CYLINDER leaf primitives (instead of composing an intersection with a separate PhiCut), addressing previously observed ray “leaks” for wedge geometries.
Changes:
- Add 5-arg
sn::ZSphere/sn::Cylinderoverloads that parkstartPhi/deltaPhiinto the param slots formerly used forcx/cy. - Update
U4Solidto emit phi-baked primitives for spheres and tubs. - Extend leaf intersection + distance logic for
CSG_ZSPHEREandCSG_CYLINDER, adjust cylinder AABB handling, and add a regression test.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| u4/U4Solid.h | Emits phi-baked ZSphere/Cylinder primitives instead of aborting/avoiding phi wedges. |
| sysrap/sn.h | Adds 5-arg Cylinder/ZSphere constructors encoding startPhi/deltaPhi into params. |
| CSG/csg_intersect_leaf_zsphere.h | Implements optional phi-wedge clipping for ZSphere leaf distance/intersection. |
| CSG/csg_intersect_leaf_cylinder.h | Implements optional phi-wedge clipping (including wedge walls) for Cylinder leaf distance/intersection. |
| CSG/CSGNode.cc | Updates cylinder AABB computation to ignore cx/cy now repurposed for phi params. |
| CSG/tests/intersect_leaf_phi_wedge_test.cc | Adds CPU regression tests for baked-in phi-wedge behavior on ZSphere/Cylinder. |
| CSG/tests/CMakeLists.txt | Registers the new regression test with CMake/ctest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (t1sph > t_min && t1sph_ok) | ||
| t_cand = t1sph; // t1sph qualified and t1cap disabled or disqualified -> t1sph | ||
| else if (t1cap > t_min) | ||
| t_cand = t1cap; // t1cap qualifies -> t1cap | ||
| else if( t2cap > t_min ) t_cand = t2cap ; // t2cap qualifies -> t2cap |
| float r_xy = sqrtf(p.x * p.x + p.y * p.y); | ||
| float d_to_edge = r_xy * fminf(dphi - deltaPhi, twoPi - dphi); |
| float r_xy = sqrtf(pos.x * pos.x + pos.y * pos.y); | ||
| float d_to_edge = r_xy * fminf(dphi - deltaPhi, twoPi - dphi); |
| auto phi_in_wedge = [&](float t) -> bool { | ||
| if (!has_phi_clip) | ||
| return true; | ||
| float x = O.x + t * D.x; // local-frame hit coords (O = origin − center) | ||
| float y = O.y + t * D.y; | ||
| float phi = atan2f(y, x); | ||
| const float twoPi = 2.f * M_PIf; | ||
| while (phi < startPhi) phi += twoPi; | ||
| float dphi = phi - startPhi; | ||
| return dphi <= deltaPhi; | ||
| }; |
| auto phi_in_wedge = [&](float xh, float yh) -> bool { | ||
| if (!has_phi_clip) | ||
| return true; | ||
| float phi = atan2f(yh, xh); | ||
| const float twoPi = 2.f * M_PIf; | ||
| while (phi < startPhi) phi += twoPi; | ||
| return (phi - startPhi) <= deltaPhi; | ||
| }; |
Two places in bool has_deltaPhi_expect = has_deltaPhi == false ; Any G4Sphere with startPhi != 0 or deltaPhi != 2π triggers assert + SIGINT. G4Tubs (init_Tubs) — silent bug. init_Tubs() on main never reads startPhi/deltaPhi from the G4Tubs at all — it passes only (rmax, -hz, hz) to sn::Cylinder, producing a full cylinder regardless. Photons leak through where the phi boundary should be. |
Summary
Phi-wedged
G4SphereandG4Tubscurrently either abort inU4Solidor rely on a composedCSG_INTERSECTION(primitive, PhiCut)representation. That composition can lose valid hits when the first primitive candidate lies outside the wedge but a later in-wedge candidate should still be returned.This PR bakes
startPhi/deltaPhidirectly into the origin-centeredZSphereandCylinderprimitives so phi clipping happens inside the primitive intersector, where candidate ordering is known.Motivation
CSG_INTERSECTION(primitive, PhiCut), where a valid later hit can be dropped if the near candidate falls outside the wedge.G4Sphere/G4Tubsto convert throughU4Solidinstead of hitting the previous abort path.What Changed
sn::ZSphere(radius, z1, z2, startPhi, deltaPhi)sn::Cylinder(radius, z1, z2, startPhi, deltaPhi)startPhi/deltaPhiin radians.csg_intersect_leaf_zsphere.hto reject sphere-surface candidates outside the phi wedge.csg_intersect_leaf_cylinder.hto:U4Solid::init_Sphere_andU4Solid::init_Tubs()to emit phi-baked primitives instead of aborting or depending on a separatePhiCutnode.cx/cyare no longer interpreted as a geometric center when those slots carry phi parameters.intersect_leaf_phi_wedge_test.ccregression coverage for recovered far hits, outside-wedge misses, and no-clip controls.Behavior / Compatibility
deltaPhi == 0ordeltaPhi >= 2pipreserves the previous full-primitive behavior.Sphere/ZSphere/Cylinder/Tubspaths are intended to be unchanged.U4Solidnow emits a phi-bakedZSphereover the full[-radius, +radius]z-range.CSG_INTERSECTION(primitive, PhiCut)and keeps wedge logic local to the primitive intersector.Test Coverage
CSG/tests/intersect_leaf_phi_wedge_test.ccThis test covers:
Scope Note
This PR mainly covers the primitive/intersector path and the corresponding
U4Solidemitters. Additional end-to-end and sliced-sphere edge-case coverage would still be useful follow-up work.