Apply custom segments when building MswSegments#758
Closed
Conversation
Iterate per-model bounding boxes and skip models outside the camera frustum to prevent distant off-screen objects from inflating the far plane. This improves depth buffer precision and reduces z-fighting. Enforce a maximum far/near ratio of 3000.0 by pushing the near plane forward if necessary. Additionally, refine the near plane adjustment when zooming into a point of interest to prevent geometry clipping.
…mand feature Use CAF_PDM_LOG_ERROR and qFatal to notify the developer with an actionable message when a command feature ID is not registered in the factory. This replaces the silent CAF_ASSERT and makes it clear that the feature may have been deleted without removing all references in appendMenuItems().
…lding RicMswSegment objects Previously, custom segment intervals were applied only during WELSEGS output generation in collectWelsegsSegment(), so RicMswSegment objects were built purely from grid cell boundaries. Completions and intersection lengths were therefore attached to full cell-crossing segments rather than to the correct custom-interval sub-segments. Add splitIntersectionsAtCustomBoundaries() to RicWellPathExportMswTableData, called after filterIntersections() in generateWellSegmentsForMswExportInfo(). For each custom interval boundary MD that falls strictly inside a WellPathCellIntersectionInfo, the intersection is split at that MD: the split point is interpolated from the well path geometry and intersectionLengthsInCellCS is recomputed using RigWellPathIntersectionTools::calculateLengthInCell, matching the pattern already used in filterIntersections(). Each resulting RicMswSegment now spans at most one custom interval, ensuring completions and intersection data are assigned correctly.
…ration Custom interval boundaries are now applied before RicMswSegment objects are created (via splitIntersectionsAtCustomBoundaries), so each segment already spans at most one custom interval by the time output is generated. The downstream WELSEGS output chain no longer needs to re-apply these boundaries. Remove the customSegmentIntervals parameter from createSubSegmentMDPairs, collectWelsegsSegment, collectValveWelsegsSegment, collectCompletionsForSegment, collectCompletionWelsegsSegments, collectWelsegsDataRecursively, and collectWelsegsData. Simplify createSubSegmentMDPairs to only subdivide by maxSegmentLength.
…gment When a custom segment interval spans multiple grid cells, the previous implementation still created one RicMswSegment per cell crossing inside that interval. Custom intervals are now honoured: consecutive cell intersections that fall within the same custom interval are grouped and create a single RicMswSegment spanning the full interval. In createWellPathSegments(), after the boundary split, cell intersections are grouped: cells in the same custom interval are merged into one SegmentGroup; cells outside all custom intervals remain as individual groups. One RicMswSegment is created per group. Perforation completions are still assigned per-cell within the group so that COMPSEGS output retains correct per-cell intersection data. The processedIntersections (boundary-split) vector replaces filteredIntersections in the assignValveContributionsToSuperXICDs call to keep consistent cell-level detail there too.
…terval segments Add isCustomInterval flag to RicMswSegment. For segments marked as custom intervals, skip maxSegmentLength subdivision in WELSEGS output and use the segment end MD as the WELSEGS length reference point instead of the cell midpoint.
Review Summary by QodoMSW: Apply custom segments at intersection level before building segments
WalkthroughsDescription• Split cell intersections at custom segment boundaries before building RicMswSegment objects - Ensures completions and intersection lengths are assigned to correct custom-interval sub-segments - Adds splitIntersectionsAtCustomBoundaries() function to process intersections early • Remove custom segment interval handling from WELSEGS output generation - Simplifies collectWelsegsSegment() by eliminating complex interval-aware subdivision logic - Custom intervals now honored as single output rows without maxSegmentLength subdivision • Improve near/far clipping plane calculation in viewer - Iterate per-model bounding boxes and skip models outside camera frustum - Enforce maximum far/near ratio of 3000.0 to preserve depth buffer precision • Enhance error handling for unknown command features - Replace silent CAF_ASSERT with clear error message and qFatal notification Diagramflowchart LR
A["Cell Intersections"] -->|splitIntersectionsAtCustomBoundaries| B["Split at Custom Boundaries"]
B -->|createWellPathSegments| C["RicMswSegment Objects"]
C -->|collectWelsegsData| D["WELSEGS Output"]
E["Custom Intervals"] -.->|define boundaries| B
F["maxSegmentLength"] -.->|no longer used in createSubSegmentMDPairs| D
File Changes1. ApplicationLibCode/Commands/CompletionExportCommands/RicMswSegment.h
|
Code Review by Qodo
1. Viewer near/far plane changes
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+604
to
+614
| // Enforce a maximum far/near ratio to preserve depth buffer precision. | ||
| // A 24-bit depth buffer has ~16 M discrete depth values; a far/near ratio of N | ||
| // means the nearest 1/N of the view depth gets half of those values. | ||
| // At 3000 the near zone still receives ~5500 depth steps per unit, which is | ||
| // sufficient for typical reservoir models. Pushing the near plane forward is | ||
| // preferable to allowing z-fighting from a degraded far plane. | ||
| const double maxFarNearRatio = 3000.0; | ||
| if ( ( *nearPlaneDist ) > 0 && ( ( *farPlaneDist ) / ( *nearPlaneDist ) ) > maxFarNearRatio ) | ||
| { | ||
| ( *nearPlaneDist ) = ( *farPlaneDist ) / maxFarNearRatio; | ||
| } |
There was a problem hiding this comment.
1. viewer near/far plane changes 📘 Rule violation ⛯ Reliability
This PR modifies camera near/far plane calculation behavior in caf::Viewer::calculateNearFarPlanes(), which is unrelated to MSW segment export logic. These changes expand PR scope and increase the chance of unintended rendering regressions.
Agent Prompt
## Issue description
This PR includes rendering/camera near/far plane logic changes in `Fwk/AppFwk/cafViewer/cafViewer.cpp` that are not directly related to applying custom segments when building `MswSegments`.
## Issue Context
To comply with minimal-diff requirements, unrelated improvements should be split into separate PRs unless explicitly required by the MSW segmentation task.
## Fix Focus Areas
- Fwk/AppFwk/cafViewer/cafViewer.cpp[604-614]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+173
to
178
| // Custom interval segments are honoured as a single output row — do not subdivide by maxSegmentLength. | ||
| const double effectiveMaxLength = segment->isCustomInterval() ? 0.0 : maxSegmentLength; | ||
| std::vector<std::pair<double, double>> segments = RicMswTableDataTools::createSubSegmentMDPairs( startMD, endMD, effectiveMaxLength ); | ||
|
|
||
| CVF_ASSERT( branch->wellPath() ); | ||
|
|
There was a problem hiding this comment.
2. Custom md uses segment end 🐞 Bug ✓ Correctness
RicMswTableDataTools::collectWelsegsSegment uses subEndMD as the WELSEGS reference/output MD for segments marked isCustomInterval, changing LENGTH/DEPTH (and diameter/roughness sampling) away from the existing midpoint convention used elsewhere in the MSW export code.
Agent Prompt
### Issue description
Custom-interval MSW segments are currently exported with `refMD = subEndMD` and `outputMD = refMD`, while the codebase documents/assumes WELSEGS segment positions are reported at segment midpoints. This changes LENGTH/DEPTH and property sampling semantics only for custom segments.
### Issue Context
The PR already correctly prevents further subdivision of custom-interval segments via `effectiveMaxLength = 0.0`. The remaining change needed is to keep the midpoint reference point for WELSEGS calculations.
### Fix Focus Areas
- ApplicationLibCode/Commands/CompletionExportCommands/RicMswTableDataTools.cpp[173-266]
- ApplicationLibCode/Commands/CompletionExportCommands/RicMswBranch.cpp[178-185]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+578
to
585
| // If the camera is inside (or past) the bounding box, allow the near plane to move | ||
| // closer to the camera based on the point of interest distance, so zooming into details | ||
| // does not clip geometry. | ||
| if ( minDistEyeToCornerAlongViewDir <= 0 && m_navigationPolicy.notNull() && m_navigationPolicyEnabled ) | ||
| { | ||
| double pointOfInterestDist = ( eye - navPointOfinterest ).length(); | ||
| ( *nearPlaneDist ) = CVF_MIN( ( *nearPlaneDist ), pointOfInterestDist * 0.2 ); | ||
| ( *nearPlaneDist ) = CVF_MAX( m_defaultPerspectiveNearPlaneDistance, pointOfInterestDist * 0.1 ); | ||
| } |
There was a problem hiding this comment.
3. Near plane formula reversed 🐞 Bug ✓ Correctness
caf::Viewer::calculateNearFarPlanes states it will move the near plane closer when the camera is inside the bounding box, but the new code sets nearPlaneDist using CVF_MAX(default, pointOfInterestDist*0.1), which cannot move the near plane closer than the previously computed value and can instead push it farther away.
Agent Prompt
### Issue description
The near-plane adjustment path meant to reduce clipping when zooming uses `CVF_MAX(...)`, which can only keep or increase the near plane distance, contradicting the stated intent of moving it closer.
### Issue Context
This logic triggers when `minDistEyeToCornerAlongViewDir <= 0` and navigation policy is enabled.
### Fix Focus Areas
- Fwk/AppFwk/cafViewer/cafViewer.cpp[575-589]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+525
to
563
| cvf::Frustum viewFrustum = rendering->camera()->frustum(); | ||
| const cvf::Scene* scene = rendering->scene(); | ||
| bool anyModelContributed = false; | ||
|
|
||
| if ( scene ) | ||
| { | ||
| for ( cvf::uint mIdx = 0; mIdx < scene->modelCount(); ++mIdx ) | ||
| { | ||
| maxDistEyeToCornerAlongViewDir = distEyeBoxCornerAlongViewDir; | ||
| const cvf::Model* model = scene->model( mIdx ); | ||
| cvf::BoundingBox modelBB = model->boundingBox(); | ||
|
|
||
| if ( !modelBB.isValid() ) continue; | ||
| if ( viewFrustum.isOutside( modelBB ) ) continue; | ||
|
|
||
| cvf::Vec3d corners[8]; | ||
| modelBB.cornerVertices( corners ); | ||
| for ( int cIdx = 0; cIdx < 8; ++cIdx ) | ||
| { | ||
| double dist = ( corners[cIdx] - eye ) * viewdir; | ||
| maxDistEyeToCornerAlongViewDir = CVF_MAX( maxDistEyeToCornerAlongViewDir, dist ); | ||
| minDistEyeToCornerAlongViewDir = CVF_MIN( minDistEyeToCornerAlongViewDir, dist ); | ||
| } | ||
| anyModelContributed = true; | ||
| } | ||
| } | ||
|
|
||
| if ( distEyeBoxCornerAlongViewDir < minDistEyeToCornerAlongViewDir ) | ||
| if ( !anyModelContributed ) | ||
| { | ||
| // Fallback: use the scene-wide bounding box (original behaviour) | ||
| cvf::BoundingBox bb = rendering->boundingBox(); | ||
| cvf::Vec3d bboxCorners[8]; | ||
| bb.cornerVertices( bboxCorners ); | ||
| for ( int bcIdx = 0; bcIdx < 8; ++bcIdx ) | ||
| { | ||
| minDistEyeToCornerAlongViewDir = distEyeBoxCornerAlongViewDir; // Sometimes negative-> behind camera | ||
| double dist = ( bboxCorners[bcIdx] - eye ) * viewdir; | ||
| maxDistEyeToCornerAlongViewDir = CVF_MAX( maxDistEyeToCornerAlongViewDir, dist ); | ||
| minDistEyeToCornerAlongViewDir = CVF_MIN( minDistEyeToCornerAlongViewDir, dist ); | ||
| } | ||
| } |
There was a problem hiding this comment.
4. Far plane can’t expand 🐞 Bug ⛯ Reliability
caf::Viewer::calculateNearFarPlanes now excludes models whose bounding boxes are outside the camera’s current frustum when computing farPlaneDist, so models beyond the current far clip are never considered and farPlaneDist cannot grow to include them unless no model contributes and the fallback path runs.
Agent Prompt
### Issue description
Far plane calculation currently filters models via `camera()->frustum()` before computing the new far plane, which can prevent farPlaneDist from expanding to include distant models beyond the current far clip.
### Issue Context
The fallback to scene-wide bounding box runs only when *no* model contributes, so the failure mode occurs when some nearer model contributes but a farther in-view model is clipped.
### Fix Focus Areas
- Fwk/AppFwk/cafViewer/cafViewer.cpp[525-563]
- Fwk/VizFwk/LibRender/cvfCamera.cpp[824-832]
- Fwk/VizFwk/LibGeometry/cvfFrustum.cpp[201-227]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.