Skip to content

Commit d748a8c

Browse files
authored
refactor(frame)!: replace legacy sframe path with stree-backed sframe (#390)
Addresses #311 by removing the legacy `WITH_OLD_FRAME` compatibility path and consolidating frame handling on the newer stree-backed frame implementation. This keeps the public frame name as `sframe`, but replaces the old metadata-heavy implementation with the lightweight double-precision frame layout that had been introduced as `sfr`. ## What Changed - Removed `WITH_OLD_FRAME` and `FRAME_TRANSITION` branches from the frame setup path. - Replaced the legacy `sframe` internals with the lightweight frame representation: - double-precision `ce`, `m2w`, `w2m`, bbox, and aux fields - direct serialization through `sframe.npy` - no old `frs`, `ekv`, tree/dynamic digest metadata, `prepare`, `populate`, or `spawn_lite` path - Deleted the transitional `sfr` header/test path: - removed `sysrap/sfr.h` - removed/renamed obsolete `sfr` tests - updated includes and type names back to `sframe` - Made `stree` the canonical frame lookup source: - `stree::get_frame*` now returns `sframe` - `CSGFoundry::getFrame*` delegates to `stree` - removed duplicate `CSGTarget::getFrame` frame construction logic - Updated `SEvt` to use simulation/tree-backed frame setup: - replaces `setGeo`/`SGeo` frame access with `setSim`/`setFr` - uses `sframe` for input photon transforms, simtrace gensteps, frame persistence, and local photon frame lookup - Updated rendering and simtrace users: - `CSGOptiX` initializes `SEvt` through `SSim` and sets render frames from `stree` - `CSGSimtrace` and `SSimtrace` use `SEvt::setFr` - `SFrameGenstep` consumes the new `sframe` layout and stores grid scale in-frame - Updated tests and helper scripts to use `sframe`, `sframe.h`, `sframe.npy`, and `SFRAME_FOLD`. ## Behavior Notes - Persisted frame output remains `sframe.npy`, but its layout is now the lightweight double-precision frame layout. - Old frame-selection metadata such as `ekv`, `tree`, and `dynamic` digest annotations is intentionally removed. - Frame lookup is now centered on `SSim/stree`, reducing duplicated CSG-level frame construction. Resolves #311
1 parent c3535fb commit d748a8c

51 files changed

Lines changed: 1128 additions & 3044 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CSG/CSGFoundry.cc

Lines changed: 25 additions & 240 deletions
Original file line numberDiff line numberDiff line change
@@ -3178,8 +3178,6 @@ CSGFoundry* CSGFoundry::Load() // static
31783178
return dst ;
31793179
}
31803180

3181-
3182-
31833181
/**
31843182
CSGFoundry::CopySelect
31853183
-------------------------
@@ -3196,9 +3194,7 @@ overriding the empty dst SSim instance.
31963194
Notice that the stree that the SSim contains is not changed by
31973195
this CSGFoundry level dynamic geometry selection, so
31983196
the stree::get_tree_digest will not change as a result of the
3199-
ELV geometry selection. Due to this issue, added stree::make_tree_digest
3200-
that is used from SSim::AnnotateFrame which includes the elv
3201-
SBitSet info into the dynamically formed digest.
3197+
ELV geometry selection.
32023198
32033199
**/
32043200

@@ -3597,218 +3593,48 @@ Grab these from remote with::
35973593
35983594
)" ;
35993595

3600-
3601-
sframe CSGFoundry::getFrame() const // TODO: MIGRATE TO sfr.h
3596+
sframe CSGFoundry::getFrame() const
36023597
{
3603-
const char* moi_or_iidx = ssys::getenvvar("MOI",sframe::DEFAULT_FRS); // DEFAULT_FRS "-1"
3598+
const char* moi_or_iidx = ssys::getenvvar("MOI", "-1");
36043599
return getFrame(moi_or_iidx);
36053600
}
36063601

36073602
/**
36083603
CSGFoundry::getFrame
36093604
--------------------
36103605
3611-
The FRAME_TRANSITION starts moving away from sframe by
3612-
switching to stree::getFrame to provide sfr which is used to
3613-
populate the sframe
3614-
3615-
Currently sframe::populate lacks handling of the grid metadata
3616-
needed for simtrace. Actually seems that metadata not much used and
3617-
anyhow if it was needed could include into the genstep metadata.
3606+
Return the lightweight frame from stree. This keeps the historical CSGFoundry
3607+
entrypoint while using the same sframe implementation as the rest of the current
3608+
frame path.
36183609
36193610
**/
36203611

3621-
3622-
sframe CSGFoundry::getFrame(const char* q_spec) const // TODO: MIGRATE TO sfr.h
3612+
sframe CSGFoundry::getFrame(const char* q_spec) const
36233613
{
3624-
sframe fr = {} ;
3625-
3626-
#ifdef FRAME_TRANSITION
3627-
stree* tree = getTree();
3614+
const stree* tree = getTree();
36283615
assert(tree);
3629-
sfr f = tree->get_frame(q_spec);
3630-
fr.populate(f);
3631-
#else
3632-
const char* arg = frs ? frs : sframe::DEFAULT_FRS ;
3633-
int rc = getFrame(fr, arg );
3634-
LOG_IF(error, rc != 0) << " arg" << arg << std::endl << getFrame_NOTES ;
3635-
if(rc != 0) std::raise(SIGINT);
3636-
#endif
3637-
fr.prepare(); // creates Tran<double>
3638-
3639-
return fr ;
3640-
}
3641-
3642-
3643-
3644-
#ifndef FRAME_TRANSITION
3645-
3646-
/**
3647-
CSGFoundry::getFrame
3648-
---------------------
3649-
3650-
3651-
FRAME_TRANSITION SEEKS TO ELIMINATE THIS
3652-
3653-
3654-
3655-
3656-
3657-
frs
3658-
frame specification string is regarded to "looks_like_moi" when
3659-
it starts with a letter or it contains ":" or it is "-1".
3660-
For such strings parseMOI is used to extract midx, mord, oodx
3661-
3662-
Otherwise the string is assumed to be inst_idx and iidxg
3663-
parsed as an integer
3664-
3665-
3666-
Q: is indexing by MOI and inst_idx equivalent ? OR: Can a MOI be converted into inst_idx and vice versa ?
3667-
3668-
* NO not for global prims : for example for all the repeated prims in the global geometry
3669-
there is only one inst_idx (zero) but there are many possible MOI
3670-
3671-
* NO not for most instanced prim, where all the prim within an instance
3672-
share the same inst_idx and transform
36733616

3674-
* BUT for the outer prim of an instance a correspondence is possible
3617+
const char* spec = q_spec ? q_spec : "-1";
36753618

3676-
TODO : AVOID DUPLICATION BETWEEN THIS AND stree::get_frame
3677-
3678-
3679-
looks_like_raw:true
3680-
frs contains "," delimiting center_extent CE values eg::
3681-
3682-
MOI=0.000,0.000,18935.000,1435.000 FULLSCREEN=0 cxr_min.sh
3683-
3684-
3685-
**/
3686-
3687-
3688-
3689-
3690-
int CSGFoundry::getFrame(sframe& fr, const char* frs ) const // TODO: MIGRATE TO sfr.h
3691-
{
3692-
3693-
bool VERBOSE = ssys::getenvbool(getFrame_VERBOSE) ;
3694-
LOG(LEVEL) << "[" << getFrame_VERBOSE << "] " << VERBOSE ;
3695-
3696-
int rc = 0 ;
3697-
3698-
bool looks_like_axis = sstr::StartsWith(frs,stree::AXIS_PFX) ;
3699-
bool looks_like_raw = strstr(frs,",") ;
3700-
bool looks_like_moi = sstr::StartsWithLetterAZaz(frs) || strstr(frs, ":") || strcmp(frs,"-1") == 0 ;
3701-
3702-
LOG_IF(info, VERBOSE)
3703-
<< "[" << getFrame_VERBOSE << "] " << ( VERBOSE ? "YES" : "NO " )
3704-
<< " frs " << ( frs ? frs : "-" )
3705-
<< " looks_like_moi " << ( looks_like_moi ? "YES" : "NO " )
3706-
<< " looks_like_raw " << ( looks_like_raw ? "YES" : "NO " )
3707-
;
3708-
3709-
if(looks_like_axis)
3710-
{
3711-
// ASSERTS WITHOUT THIS FROM TOO MANY ELEM IN PopulateFromRaw
3712-
// BUT SUSPECT THE sframe NOT REALLY USED
3713-
//
3714-
// ABOVE IS NOT TRUE, THE PERSISTED sframe STILL USED FROM CSGOptiX/cxt_min.py for simtrace plotting
3715-
// SO STILL MUST DO THINGS IN DUPLICATE
3716-
sfr lf = sfr::MakeFromAxis<float>(frs + strlen(stree::AXIS_PFX), ',');
3717-
fr.populate(lf);
3718-
}
3719-
else if(looks_like_raw)
3619+
if (sstr::IsInteger(spec))
37203620
{
3721-
rc = sframe::PopulateFromRaw(fr, frs, ',' );
3621+
std::string inst_spec = stree::INST_PFX;
3622+
inst_spec += spec;
3623+
return tree->get_frame(inst_spec.c_str());
37223624
}
3723-
else if(looks_like_moi)
3724-
{
3725-
int midx, mord, gord ; // mesh-index, mesh-ordinal, gas-ordinal
3726-
parseMOI(midx, mord, gord, frs );
3727-
3728-
rc = getFrame(fr, midx, mord, gord);
3729-
// NB gas ordinal is not the same as gas index
3730-
3731-
LOG_IF(info, VERBOSE)
3732-
<< "[" << getFrame_VERBOSE << "] " << ( VERBOSE ? "YES" : "NO " )
3733-
<< " frs " << ( frs ? frs : "-" )
3734-
<< " looks_like_moi " << ( looks_like_moi ? "YES" : "NO " )
3735-
<< " midx " << midx
3736-
<< " mord " << mord
3737-
<< " gord " << gord
3738-
<< " rc " << rc
3739-
;
3740-
3741-
}
3742-
else
3743-
{
3744-
int inst_idx = SName::ParseIntString(frs, 0) ;
3745-
rc = getFrame(fr, inst_idx);
37463625

3747-
LOG_IF(info, VERBOSE)
3748-
<< "[" << getFrame_VERBOSE << "] " << ( VERBOSE ? "YES" : "NO " )
3749-
<< " frs " << ( frs ? frs : "-" )
3750-
<< " looks_like_moi " << ( looks_like_moi ? "YES" : "NO " )
3751-
<< " inst_idx " << inst_idx
3752-
<< " rc " << rc
3753-
;
3754-
}
3755-
3756-
fr.set_propagate_epsilon( SEventConfig::PropagateEpsilon() );
3757-
fr.frs = strdup(frs);
3758-
fr.prepare(); // needed for spawn_lite to work
3759-
3760-
LOG_IF(info, VERBOSE)
3761-
<< "[" << getFrame_VERBOSE << "] " << ( VERBOSE ? "YES" : "NO " )
3762-
<< "[fr.desc\n"
3763-
<< fr.desc()
3764-
<< "]fr.desc\n"
3765-
;
3766-
3767-
LOG_IF(error, rc != 0) << "Failed to lookup frame with frs [" << frs << "] looks_like_moi " << looks_like_moi ;
3768-
return rc ;
3626+
return tree->get_frame(spec);
37693627
}
3770-
#endif
3771-
37723628

37733629
int CSGFoundry::getFrame(sframe& fr, int inst_idx) const
37743630
{
3775-
return target->getFrame( fr, inst_idx );
3776-
}
3777-
3778-
/**
3779-
CSGFoundry::getFrame
3780-
----------------------
3781-
3782-
midx
3783-
mesh index (aka lv)
3784-
mord
3785-
mesh ordinal (picking between multipler occurrences of midx
3786-
gord
3787-
GAS ordinal [NB this is not the GAS index]
3788-
3789-
3790-
NB the GAS index is determined from (midx, mord)
3791-
and then gord picks between potentially multiple occurrences
3792-
3793-
**/
3631+
const stree* tree = getTree();
3632+
assert(tree);
37943633

3795-
int CSGFoundry::getFrame(sframe& fr, int midx, int mord, int gord) const
3796-
{
3797-
int rc = 0 ;
3798-
if( midx == -1 )
3799-
{
3800-
unsigned ias_idx = 0 ; // only one IAS
3801-
unsigned long long emm = 0ull ; // hmm instance var ?
3802-
iasCE(fr.ce, ias_idx, emm);
3803-
}
3804-
else
3805-
{
3806-
rc = target->getFrame( fr, midx, mord, gord );
3807-
}
3808-
return rc ;
3634+
fr = tree->get_frame_inst(inst_idx);
3635+
return 0;
38093636
}
38103637

3811-
38123638
/**
38133639
CSGFoundry::getFrameE
38143640
-----------------------
@@ -3827,63 +3653,39 @@ see CSGFoundry::getFrameE. Possible envvars include:
38273653
+------------------------------+----------------------------+
38283654
38293655
3830-
The sframe::set_ekv records into frame metadata the envvar key and value
3831-
that picked the frame.
3832-
3833-
3834-
Q: WHY NOT DO THIS AT LOWER LEVEL ?
3835-
A: Probably because it needs getFrame and it predates the stree.h reorganization
3836-
that made frame access at sysrap level possible.
3837-
3838-
3839-
NB this is called both by the below CSGFoundry::AfterLoadOrCreate and by CSGOptiX::initFrame
3840-
so that should mean that frame annotation always gets done for running from
3841-
persisted or live geometry (HMM perhaps called twice though)
3656+
The old frame path recorded metadata describing which environment variable
3657+
selected the frame. That metadata is intentionally not preserved for sframe.
38423658
38433659
**/
38443660

3845-
3846-
38473661
sframe CSGFoundry::getFrameE() const
38483662
{
38493663
bool VERBOSE = ssys::getenvbool(getFrameE_VERBOSE) ;
38503664
LOG(LEVEL) << "[" << getFrameE_VERBOSE << "] " << VERBOSE ;
38513665

3852-
sframe fr = {} ;
3853-
38543666
if(ssys::getenvbool("INST"))
38553667
{
38563668
int INST = ssys::getenvint("INST", 0);
38573669
LOG_IF(info, VERBOSE) << " INST " << INST ;
3670+
sframe fr = {};
38583671
getFrame(fr, INST ) ;
3859-
3860-
fr.set_ekv("INST");
3672+
return fr;
38613673
}
38623674
else if(ssys::getenvbool("MOI"))
38633675
{
38643676
const char* MOI = ssys::getenvvar("MOI", nullptr) ;
38653677
LOG_IF(info, VERBOSE) << " MOI " << MOI ;
3866-
fr = getFrame() ;
3867-
fr.set_ekv("MOI");
3678+
return getFrame();
38683679
}
38693680
else
38703681
{
38713682
const char* ipf_ = SEventConfig::InputPhotonFrame(); // OPTICKS_INPUT_PHOTON_FRAME
38723683
const char* ipf = ipf_ ? ipf_ : "0" ;
38733684
LOG_IF(info, VERBOSE) << " ipf " << ipf ;
3874-
fr = getFrame(ipf);
3875-
3876-
fr.set_ekv(SEventConfig::kInputPhotonFrame, ipf );
3685+
return getFrame(ipf);
38773686
}
3878-
3879-
3880-
SSim::AnnotateFrame(fr, elv, "CSGFoundry::getFrameE" ); // set tree and dynamic digests into the frame
3881-
3882-
return fr ;
38833687
}
38843688

3885-
3886-
38873689
/**
38883690
CSGFoundry::AfterLoadOrCreate
38893691
-------------------------------
@@ -3900,31 +3702,14 @@ Formerly frame mechanics had to be done up here at CSGFoundry level
39003702
due to the need for geometry info to form the frame from envvar config.
39013703
But after stree.h improvements there is no reason not to do within SSim+stree
39023704
3903-
TODO: reposition SEvt prep into SSim::AfterLoadOrCreate and sframe/sfr prep into stree::AfterLoadOrCreate ?
3904-
3905-
3906-
3907-
Trying to progress with FRAME_TRANSITION by replacing the old
3908-
CSGFoundry::AfterLoadOrCreate with SSim::afterLoadOrCreate
3705+
TODO: reposition SEvt prep into SSim::AfterLoadOrCreate and frame prep into stree::AfterLoadOrCreate ?
39093706
39103707
39113708
**/
39123709

39133710
void CSGFoundry::AfterLoadOrCreate() // static
39143711
{
39153712
assert(0 && "DONT USE THIS : THIS PREP DONE BY SSim::afterLoadOrCreate " );
3916-
3917-
CSGFoundry* fd = CSGFoundry::Get();
3918-
if(!fd) return ;
3919-
3920-
#ifdef WITH_OLD_FRAME
3921-
SEvt::CreateOrReuse() ; // creates 1/2 SEvt depending on OPTICKS_INTEGRATION_MODE
3922-
3923-
sframe fr = fd->getFrameE() ;
3924-
LOG(LEVEL) << fr ;
3925-
SEvt::SetFrame(fr); // now only needs to be done once to transform input photons
3926-
#endif
3927-
39283713
}
39293714

39303715

0 commit comments

Comments
 (0)