Repository navigation
Fix crashes in ensemble fracture statistics and camera zoom all - #14973
Conversation
Conductivity and width grids were built as separate lists; a missing width grid shortened one list and widthGrids[i] went out of range. Build the grids per definition so they stay paired. Crash stack (signature 642357244e71): [3] cvf::cref<RigFractureGrid>::cref(cvf::cref<RigFractureGrid> const&) at cvfObject.inl:410 [4] RigEnsembleFractureStatisticsCalculator::calculateProperty(...) at RigEnsembleFractureStatisticsCalculator.cpp:156 [5] RimStimPlanFractureTemplate::generatePropertiesTable() const at RimStimPlanFractureTemplate.cpp:791 [6] RimStimPlanFractureTemplate::loadDataAndUpdate() at RimStimPlanFractureTemplate.cpp:221 [7] RicMeshFractureTemplateHelper<RimStimPlanFractureTemplate>::createNewTemplatesFromFiles(...) at RicMeshFractureTemplateHelper.inl:149 [8] RicMeshFractureTemplateHelper<RimStimPlanFractureTemplate>::createNewTemplates(...) at RicMeshFractureTemplateHelper.inl:89 [9] RicNewStimPlanFractureTemplateFeature::createNewTemplates() at RicNewStimPlanFractureTemplateFeature.cpp:58 [10] RicNewStimPlanFractureTemplateFeature::onActionTriggered(bool) at RicNewStimPlanFractureTemplateFeature.cpp:78
181c511 to
e86a8fa
Compare
kriben
left a comment
There was a problem hiding this comment.
Reviewed all 4 changed files using open-code-review-delegate for file selection and rule resolution (100% coverage, no skipped files). Found two medium-severity gaps in the camera fix, detailed inline. All 4 camera tests passed against the PR's camera source linked with existing framework libraries; both additional cases below were reproduced separately. No actionable issue found in the fracture-grid pairing change. Full application build and fracture tests were not run.
| Vec3d Camera::computeFitViewEyePosition(const BoundingBox& boundingBox, const Vec3d& dirIn, const Vec3d& up, double coverageFactor, double fieldOfViewYDeg, double aspectRatio) | ||
| { | ||
| // A zero or undefined view direction cannot define a view; fall back to looking along the Y axis | ||
| cvf::Vec3d dir = (dirIn.length() > 0.0) ? dirIn : cvf::Vec3d::Y_AXIS; |
There was a problem hiding this comment.
[Medium][bug] Undefined directions bypass the fallback and still assert.
dirIn.length() > 0.0 does not reject cvf::Vec3d::UNDEFINED: its components use the maximum-double sentinel, so its length overflows to infinity and this condition evaluates true. Subsequent normalization produces degenerate vectors.
I reproduced an assertion in Plane::distanceToOrigin() by calling computeFitViewEyePosition() with Vec3d::UNDEFINED, a valid bounding box, and Vec3d::Z_AXIS as up. Please explicitly reject undefined/non-finite directions before selecting the fallback and add this input to the regression test.
There was a problem hiding this comment.
Fixed: undefined and non-finite directions now fall back to the Y axis. Test covers Vec3d::UNDEFINED.
| Vec3d Camera::computeFitViewEyePosition(const BoundingBox& boundingBox, const Vec3d& dirIn, const Vec3d& up, double coverageFactor, double fieldOfViewYDeg, double aspectRatio) | ||
| { | ||
| // A zero or undefined view direction cannot define a view; fall back to looking along the Y axis | ||
| cvf::Vec3d dir = (dirIn.length() > 0.0) ? dirIn : cvf::Vec3d::Y_AXIS; |
There was a problem hiding this comment.
[Medium][bug] Apply the direction fallback consistently in fitView().
This fallback is local to computeFitViewEyePosition(), but fitView() computes correctedUp beforehand using the original direction. For a zero direction, that up vector is zero. This helper now returns a valid eye position, but fitView() subsequently passes the zero up vector to setFromLookAt(), producing a singular camera matrix.
I reproduced this with fitView(bb, Vec3d::ZERO, Vec3d::Z_AXIS, 0.9) and a valid bounding box/viewport: the resulting direction, up, and right vectors all have length zero. Please sanitize the direction before deriving the up vector in fitView(), so eye calculation and final orientation use the same valid basis, and test the resulting camera basis rather than only the returned eye position.
There was a problem hiding this comment.
Fixed: fitView() sanitizes the direction before deriving the up vector. New test checks the resulting direction/up/right basis for zero and undefined directions.
Zoom All could pass a zero view direction (eye equal to view reference point). The up vector then also became zero, giving a plane with a zero normal and an assert in Plane::distanceToOrigin. Fall back to the Y axis when the direction is zero or undefined. Follow-up to the parallel dir/up fix in #14849. Call stack: [5] cvf::AssertHandlerConsole::handleAssert(char const*, int, char const*, char const*) at cvfAssert.cpp:125 [6] cvf::Plane::distanceToOrigin() const at cvfPlane.cpp:313 [7] cvf::Camera::computeFitViewEyePosition(cvf::BoundingBox const&, cvf::Vector3<double> const&, cvf::Vector3<double> const&, double, double, double) at cvfCamera.cpp:361 [8] caf::Viewer::zoomAll() at cafViewer.cpp:992 [9] caf::CmdFeature::actionTriggered(bool) at cafCmdFeature.cpp:176
4657a30 to
6307a7b
Compare
Fix crash when width data is missing in ensemble fracture statistics
Problem: conductivity and width grids were built as separate lists; a missing width grid shortened one list and
widthGrids[i]went out of range.Fix: build grids per definition so they stay paired, and skip definitions missing either grid.
Crash stack (signature 642357244e71, 3 reports)
Fix crash in Camera::computeFitViewEyePosition with zero view direction
Problem:
zoomAll()can pass a zero view direction; the up vector becomes zero andPlane::distanceToOriginasserts. Regression of the parallel dir/up fix in #14849 (4 reports on 2026.09.2-dev.03).Fix: fall back to the Y axis when the direction is zero or undefined.
Crash stack (signature c5301dc57661, 7 reports)