"I'm looking to consolidate and optimize the povray code base
https://github.com/POV-Ray/povray  Can you identify and summarize redundant code
that performs the same task?  Splines, bounding, etc?"

Absolutely heres a firstpass redundancy map of the POVRay codebase focused on
places where the same (or very similar) functionality is implemented in multiple
places, plus concrete refactor suggestions you can take to consolidate and speed
up maintenance. Ive grouped the hotspots by theme (splines, bounding,
roots/Sturm, sampling/jitter, etc.) and pointed to the exact files or issues
that show the overlap.

> TL;DR quick wins
> **(1)** Centralize *all* spline math in  core/math/spline.*  and have  lathe ,  sor
,  prism , and  bezier  call into
 it.
> **(2)** Route *all* primitive bounding through the  core/bounding  layer (AABB,
sphere, cylinder) and remove adhoc in
ternal bounding in shapes (esp.  sphere_sweep ).
> **(3)** Use the single polynomial solver ( polynomialsolver.* ) behind every shape
that exposes sturm toggles, and ma
ke the quality switch a uniform parameter instead of scattered  #define s per
primitive.

---

## 1) Splines: several parallel implementations

**Whats duplicated**
- POVRay already has a general spline implementation in  core/math/spline.cpp
and the file header itself notes it currently contains only stuff for the SDLs
functionalike spline feature but is *meant* to absorb spline math from
primitives that still carry their own code. Thats a strong hint that there is
duplication to retire.
- Primitive modules each embed their own spline pieces:
  - **SOR (surface of revolution)** computes cubic spline coefficients of
\(r^2=f(w)\) segmentbysegment inside  source/core/shape/sor.cpp .
  - **PRISM** declares its own spline modes ( LINEAR_SPLINE ,  QUADRATIC_SPLINE
,  CUBIC_SPLINE ,  BEZIER_SPLINE ) and handles them locally in  prism.* .
  - **LATHE** has its own parser and spline handling (linear, quadratic, cubic,
Bzier) separate from the core  spline.* .
  - Theres also a **standalone Bzier** primitive ( shape/bezier.* ) with its own
math.

**Why it matters**
- Bugs and feature requests keep landing around spline behavior (e.g., *negative
control points for lathe Bzier*, *direction/tangent evaluation*, *prism Bzier
artifacts*), and they have to be touched in multiple places. A single spline
kernel would reduce that surface area.

**Consolidation plan**
- Extend  core/math/spline.*  with a small polymorphic layer that covers
**Bzier**, **CatmullRom**, **Hermite**, **linear/quadratic/cubic** variants and
exposes **Evaluate**, **Derivative(order)**, and **LocalBBox(t0,t1)**. Then:
  - Switch  lathe ,  sor , and  prism  to construct a core  Spline  object
instead of rolling their own persegment math.
  - Implement the longstanding **spline derivative** request (#115) in the core
so SDL and geometric users get the same derivative/tangent semantics.

---

## 2) Bounding: multiple mechanisms that overlap (and sometimes conflict)

**Whats duplicated**
- There are three distinct *bounding shape* implementations**AABB**, **sphere**,
**cylinder**in  core/bounding , *and* many primitives still compute or carry
their own special bounding logic instead of deferring to the common layer.
- ** sphere_sweep ** performs internal bounding with its own heuristics (radius
multipliers) while the engine also has global bounding; several issues document
problems triggered by this duplication and precedence.
- Quadrics and CSG have specialized or optimized bounding paths and specialcases
(e.g., inverted quadrics) that diverged over time, leading to inconsistent
results.
- Userprovided ** bounded_by ** and **+/-MB** controls regressed in 3.7.x/3.8
due to code paths that use  Intersect_BBox()  without considering the configured
bounding methodagain, the same idea implemented in multiple places.
- The **bounding cylinder** type is used by lathe and sor, but those shapes
still have local extent logic; better to route both through the shared cylinder
code.

**Why it matters**
- Conflicting logic has caused uservisible bugs (e.g., quadric disappearing with
CSG when clip bounds extend, or user bounding not following transforms), all of
which become easier to fix and test if there is a single path.

**Consolidation plan**
- Make ** ObjectBase::Compute_BBox() ** a thin adapter that always builds via
the types in  core/bounding  (AABB, sphere, cylinder, BSP), and delete
shapelocal hedges where possible (esp.  sphere_sweep ). Provide *hooks* for
shapespecific inflations but centralize the policy.
- Fix all call sites to **respect  boundingMethod ** (the PR restoring 3.6
behavior flagged that several paths still ignored it) and unify
Intersect_BBox()  usage through a small wrapper that carries the method.
- Normalize **CSG quadric specialcases** in one place (core) so
intersection/difference with inverted shapes gets the same AABB logic as the
noninverted shapes.
- Ensure ** bounded_by  transforms propagate** consistently from parse to shape
to bounding; the original inconsistency is documented.

---

## 3) Root finding / Sturm toggles: solver is centralized, usage is not

**Whats duplicated**
- POVRay has a comprehensive polynomial solver in  core/math/polynomialsolver.*
, but sturm handling and solver selection is scattered across primitives ( prism
,  lathe ,  ovus , etc.) with pershape flags and defines ( STURM_OK_OBJECT ,
shapelocal logic), instead of a uniform solver API.
- Bugs and workarounds around Bzier prisms/lathe often end up recommending
sturm , highlighting that the shapes are partially reimplementing solver
choices.

**Consolidation plan**
- Provide a **single solver faade** (e.g.,  SolvePolynomial(params, mode)  where
 mode = {default|sturm|hybrid} ) in  polynomialsolver.*  and have all shapes
call it; **remove** the pershape defines/macros for sturm ok.
- Add a shared **robustness policy** (tolerances, root merging) in the solver
and delete replicated numeric tweaks in individual shapes.

---

## 4) Randomness, jitter, and sampling: different randoms in different places

**Whats duplicated**
- There are *two* math utilities here:  core/math/jitter.*  and
core/math/randomsequence.* , while sampling decisions also live in AA and light
code that dont consistently use one common RNG sequence or seeding scheme.
- In practice this shows up as reproducibility gaps:
  - **Partial render region vs full render** differences with AA on ( +am1 /
+am3 ) suggest perfeature subpixel offsets and/or RNG sequence mismatches.
  - **Area light weighting** quirks also indicate bespoke sampling math living
with the light instead of a shared sampler.
-  TraceThreadData  exposes a random sequence to features, but many paths still
sample directly.

**Consolidation plan**
- Introduce a **single sampling gateway** (e.g.,  Sampler  built on
RandomSequence ) with deterministic seeding from  TraceThreadData  and remove
adhoc calls to jitter or  rand . Migrate AA, area_lights, media scattering,
etc., to this API.
- Store AA subpixel pattern and arealight grids centrally to fix the
fullvspartial render mismatch.

---

## 5) Bounding data structures & calls duplicated across subsystems

**Whats duplicated**
- RayAABB tests and mailbox/BSP plumbing show up in multiple modules (AABB tree,
BSP tree, object intersection paths), and  Intersect_BBox()  gets called from
several places without a single policy gate (ties back to 2).

**Consolidation plan**
- Provide a **unified should we test this bbox** inline that wraps the perscene
method (+/-MB, threshold) so every call site applies the same rule. Clean up all
callers.

---

## 6) Noise & patterns: mostly centralized, but keep an eye on overlap

**Observation**
- Turbulence/Perlin/cellular plumbing is centralized in  core/material/noise.cpp
, while pattern definitions live in  core/material/pattern.h . This is in decent
shape, but if you find any perpattern local hash/gradient tables, fold them back
into  noise.cpp .

---

## 7) Platform/build config macros repeated across modules

**Observation**
- Platform detection and 64bit type selection have similar logic across multiple
 syspovconfig*.h  files; theres an open issue about misusing POSIX macrosanother
case of logic spread out and replicated per module. Consider one canonical
detection header and have module configs include it.

---

## Concrete refactor tasks (actionable list)

1) **Core Spline Kernel**
   - Extend  core/math/spline.*  to support all spline types used by shapes
(linear, quadratic, cubic, Bzier) + derivatives; port  lathe ,  sor ,  prism ,
bezier  to use it. Add unit tests with the existing Bezier/lathe and prism
scenes from issues.

2) **Bounding Unification**
   - Replace shapelocal bounding with calls to  core/bounding/*  (AABB, sphere,
cylinder). Pull  sphere_sweep s radius2 heuristic into a single tunable policy
and document it. Ensure  bounded_by  transforms propagate through the object
transform chain.

3) **Sturm/Root Solver API**
   - Create a solver faade in  polynomialsolver.*  with a consistent  sturm
toggle and tolerances; remove pershape defines such as  STURM_OK_OBJECT  in
prism.h . Re-test the prism/bezier artifacts that currently require  sturm .

4) **Sampler/RNG Gateway**
   - Add  Sampler  on top of  RandomSequence , seed via  TraceThreadData , and
convert AA, area lights, media, and jitter paths to use it. Verify determinism
for partial vs fullframe renders.

5) **CSG & Quadric Bounding Fixes**
   - Move quadric infinite cylinder AABB specialcases into core bounding and
correct the invertedshape logic. Add a regression scene for the
paraboloid/cylinder disappearance.

6) **One policy for  Intersect_BBox() **
   - Add a thin policy wrapper that checks the scenes  boundingMethod  before
doing any bbox test; convert all callers. This completes the intent of restore
3.6 user bounding control everywhere.

---

## Test scaffolding you can reuse

- **Spline**: Use the lathe/prism test bundles referenced in #60 plus any Bzier
text paths (the issue where  sturm  fixed prism artifacts). These are perfect
for regression after centralization.
- **Bounding**: Reproduce the quadric + cylinder CSG from #128 and the
sphere_sweep  scenes from #59/#288; add asserts on the computed AABBs before and
after transforms and with  -MB  toggles.
- **Sampling**: Re-run the partialregion vs fullframe AA test from #377 to
verify identical output; add area light grid comparisons from #224.

---

## Effort/Risk notes

- **Low risk / high gain**: Centralizing solver API (no behavior change unless
you flip the default), adding  Intersect_BBox  policy wrapper, and wiring shapes
to core bounding types (preserving their current inflation heuristics as
parameters).
- **Moderate risk**: Porting  lathe / sor / prism  to the core spline
kernelwatch for offbyepsilon differences at segment boundaries; add derivative
support at the same time (requested in #115) so you dont have to touch the
shapes again.
- **Moderate risk**: Sampler unification may change randomness unless you lock
seeds; provide  #version 3.8 guarded defaults as you do so.

---

## Want me to draft the first PRs?

If youd like, I can start with a **minimal, nobehaviorchange** PR that:
1) Adds the ** Intersect_BBox  policy wrapper** + touches obvious call sites,
2) Introduces a **solver faade** that proxies to current internals, and
3) Extracts a tiny spline ** Evaluate/Derivative ** pair in  core/math/spline.*
and ports **one** primitive (say,  prism ) to use it behind a compiletime
switch.

Which area do you want to prioritize first for consolidation - **splines** or
**bounding**?

Post a reply to this message