Skip to content

fix: preserve stream state in surface output#5589

Open
hadronic-arvind wants to merge 1 commit into
acts-project:mainfrom
hadronic-arvind:codex/to-stream-ostream-state
Open

fix: preserve stream state in surface output#5589
hadronic-arvind wants to merge 1 commit into
acts-project:mainfrom
hadronic-arvind:codex/to-stream-ostream-state

Conversation

@hadronic-arvind

@hadronic-arvind hadronic-arvind commented Jun 14, 2026

Copy link
Copy Markdown

Refs #3079

Summary

  • Add OstreamStateGuard to restore std::ostream flags, precision, width, and fill after formatted output.
  • Use the guard in the affected toStream / operator<< paths for surface bounds, volume bounds, Surface, SurfaceArray, PerigeeSurface, and GSF component merging output.
  • Add regression coverage that verifies stream formatting state is preserved across the updated output implementations.

Validation

  • docker exec 7c8454e52c16 cmake --build /workspaces/acts/build_codex --target ActsUnitTestSurfaceBounds ActsUnitTestSurface ActsUnitTestSurfaceArray ActsUnitTestPerigeeSurface ActsUnitTestVolumeBounds ActsUnitTestGsfComponentMerging ActsCore_HEADERS -j 4
  • docker exec 7c8454e52c16 ctest --test-dir /workspaces/acts/build_codex -R "^(SurfaceBounds|Surface|SurfaceArray|PerigeeSurface|VolumeBounds|GsfComponentMerging)$" --output-on-failure
  • pre-commit hook suite on the amended commit

@github-actions github-actions Bot added this to the next milestone Jun 14, 2026
@github-actions github-actions Bot added the Component - Core Affects the Core module label Jun 14, 2026

@benjaminhuth benjaminhuth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@hadronic-arvind

Copy link
Copy Markdown
Author

Could a maintainer please rerun merge-sentinel / trigger the required CI for this PR ?

Current head is d6dc2d959541aa355e74dd62cc47b31ed12bfc5d.
merge-sentinel is failing, and CI Bridge is still not running the downstream required CI. Earlier this was reported as:

Pipeline refused: No pipeline was triggered for this user

Local validation for this head passed:

  • file-scoped/pre-commit hook suite
  • focused build targets
  • focused CTest coverage for the changed stream-output paths

Thanks!

@andiwand

Copy link
Copy Markdown
Contributor

/ci-bridge-run

Comment on lines +43 to +47
std::ostream& m_stream;
std::ios_base::fmtflags m_flags;
std::streamsize m_precision;
std::streamsize m_width;
char m_fill;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::ostream& m_stream;
std::ios_base::fmtflags m_flags;
std::streamsize m_precision;
std::streamsize m_width;
char m_fill;
std::ostream* m_stream{};
std::ios_base::fmtflags m_flags{};
std::streamsize m_precision{};
std::streamsize m_width{};
char m_fill{};
  • std::ostream* m_stream more a matter of taste but ref members are a bit annoying sometimes
  • I would init all the fields for safety

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should rather go into some detail/ directory and namespace. I don't think we want to ship this as public API

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Will wait for the checks to be completed, make these changes and update the PR.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

📊: Physics performance monitoring for d6dc2d9

Full contents

physmon summary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component - Core Affects the Core module Track Fitting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants