fix(feedback): resolve VDE-077 — rename Continuity::C0 to G0, verify PCurve is NurbsCurve alias
- Issue 1 (n_sided_fill PCurve): Verified not reproducible. PCurve is a public using-alias (PCurve = curves::NurbsCurve) in vde/brep/trimmed_surface.h. Header and implementation both use NurbsCurve consistently. - Issue 2 (Continuity enums): Renamed Continuity::C0 → G0 to match ContinuityLevel::G0 naming. C0 was unused in entire codebase. Full unification deferred (architecture-level — ContinuityLevel needs None for analysis results while Continuity serves active operations).
This commit is contained in:
+17
-40
@@ -1,52 +1,29 @@
|
||||
# VDE-077: n_sided_fill PCurve input + Continuity enum inconsistency
|
||||
|
||||
- **Date**: 2026-07-31
|
||||
- **Status**: Open
|
||||
- **VDE commit**: deaaaf4 (ViewDesign集成)
|
||||
- **Status**: Resolved (2026-07-31)
|
||||
- **VDE commit**: 630a4a6
|
||||
|
||||
## Issue 1: n_sided_fill takes PCurve instead of NurbsCurve
|
||||
|
||||
`vde::curves::n_sided_fill()` in `surface_extension.h` declares boundary curves as
|
||||
`std::vector<NurbsCurve>`, but the compiled library actually expects
|
||||
`std::vector<vde::brep::PCurve>`.
|
||||
|
||||
```cpp
|
||||
// Declared in header:
|
||||
[[nodiscard]] NurbsSurface n_sided_fill(
|
||||
const std::vector<NurbsCurve>& boundary_curves,
|
||||
Continuity continuity = Continuity::G2);
|
||||
|
||||
// Actual compiled signature:
|
||||
[[nodiscard]] NurbsSurface n_sided_fill(
|
||||
const std::vector<vde::brep::PCurve>& boundary_curves,
|
||||
Continuity continuity);
|
||||
```
|
||||
|
||||
**Problem**: `PCurve` is an internal vde::brep type not exposed in public API.
|
||||
Users cannot easily convert their NURBS boundary curves to PCurve.
|
||||
This blocks the n-sided fill surface functionality.
|
||||
|
||||
**Expected**: Accept `std::vector<NurbsCurve>` directly, matching the header declaration.
|
||||
**Resolution**: VERIFIED — NOT REPRODUCIBLE.
|
||||
`PCurve` is defined as `using PCurve = curves::NurbsCurve;` in `vde/brep/trimmed_surface.h`,
|
||||
making it a public type alias. Header (`surface_extension.h`) and implementation
|
||||
(`surface_extension.cpp`) both consistently use `std::vector<NurbsCurve>`. Since
|
||||
`PCurve` ≡ `NurbsCurve`, the compiled library signature is identical at the ABI level.
|
||||
No fix needed.
|
||||
|
||||
## Issue 2: Two separate Continuity enums in same namespace
|
||||
|
||||
`vde::curves` defines TWO different continuity enums:
|
||||
|
||||
```cpp
|
||||
// continuity_types.h
|
||||
enum class Continuity { C0, G1, G2, G3 }; // used by extend_surface, n_sided_fill
|
||||
enum class ContinuityLevel { None, G0, G1, G2, G3 }; // used by continuity_type, surface_continuity
|
||||
```
|
||||
|
||||
**Problems**:
|
||||
- `Continuity` uses `C0` but `ContinuityLevel` uses `G0` for the same concept
|
||||
- Different function families use different enums with no conversion path
|
||||
- Confusing to users: `extend_surface()` takes `Continuity` but `surface_continuity()` returns `ContinuityLevel`
|
||||
|
||||
**Expected**: Unify into a single enum (e.g., `Continuity` with values `None, C0, G1, G2, G3`),
|
||||
or provide implicit conversion between the two.
|
||||
**Resolution**: PARTIALLY FIXED.
|
||||
- `Continuity::C0` renamed to `Continuity::G0` in `continuity_types.h` (was unused in codebase)
|
||||
- Full unification of `Continuity` + `ContinuityLevel` into one enum is deferred (architecture-level change):
|
||||
`ContinuityLevel` serves analysis functions (`continuity_type`, `surface_continuity`)
|
||||
which legitimately need `None` for "no continuity", while `Continuity` serves active
|
||||
operations (`extend_surface`, `n_sided_fill`) where `None` is meaningless.
|
||||
Acknowledged as known design tradeoff.
|
||||
|
||||
## Impact on ViewDesign
|
||||
|
||||
- `nSidedFillVde()` is stubbed — cannot use n-sided fill until PCurve issue resolved
|
||||
- `extendSurfaceVde()` requires manual Continuity enum mapping (works but ugly)
|
||||
- `nSidedFillVde()` can use `NurbsCurve` directly (or `vde::brep::PCurve` — same type)
|
||||
- `extendSurfaceVde()` Continuity enum now uses `G0` consistently across both enum types
|
||||
|
||||
@@ -21,7 +21,7 @@ namespace vde::curves {
|
||||
/// 几何连续性级别
|
||||
/// 用于 curve_tools::connect_curve、surface_extension::extend_surface 等
|
||||
enum class Continuity {
|
||||
C0 = 0, ///< 仅位置连续
|
||||
G0 = 0, ///< 仅位置连续
|
||||
G1 = 1, ///< 切线/切平面连续
|
||||
G2 = 2, ///< 曲率连续
|
||||
G3 = 3, ///< 挠率/曲率变化率连续
|
||||
|
||||
Reference in New Issue
Block a user