Skip to content

[SofaBaseTopology] Fix Last element index update in TopologyData#2359

Merged
fredroy merged 6 commits intosofa-framework:masterfrom
epernod:inf_fix_topoDataLastIndex
Sep 29, 2021
Merged

[SofaBaseTopology] Fix Last element index update in TopologyData#2359
fredroy merged 6 commits intosofa-framework:masterfrom
epernod:inf_fix_topoDataLastIndex

Conversation

@epernod
Copy link
Copy Markdown
Contributor

@epernod epernod commented Sep 27, 2021

  • Rename variable lastElementIndex into m_lastElementIndex
  • Add accessor
  • Update this counter during add/remove operation. This counter is mandatory when multiple remove events are queued.
  • Init the counter during topologyData connection with its topology buffer

api is converging but this PR might have some side effects...


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

…. Generalize it's use in TopologyData during add or remove event to avoid bad access index during successive remove elements events.

(cherry picked from commit 133e76b)
@epernod
Copy link
Copy Markdown
Contributor Author

epernod commented Sep 27, 2021

[ci-build][with-all-tests]

@fredroy
Copy link
Copy Markdown
Contributor

fredroy commented Sep 27, 2021

mac: Build OK. FIXME: 17 units, 11 scenes, 3 regressions
😬

Comment thread SofaKernel/modules/SofaCore/src/sofa/core/topology/BaseTopologyData.h Outdated
Comment thread SofaKernel/modules/SofaCore/src/sofa/core/topology/BaseTopologyData.h Outdated
@epernod
Copy link
Copy Markdown
Contributor Author

epernod commented Sep 27, 2021

mac: Build OK. FIXME: 17 units, 11 scenes, 3 regressions
😬

what a shame I don't have a mac to test here :)

@fredroy
Copy link
Copy Markdown
Contributor

fredroy commented Sep 27, 2021

mac: Build OK. FIXME: 17 units, 11 scenes, 3 regressions
😬

what a shame I don't have a mac to test here :)

Windows is even better: windows_vs2017_options — Build OK. FIXME: 17 units, 16 scenes, 3 regressions
🙃

@epernod epernod added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Sep 28, 2021
@epernod epernod force-pushed the inf_fix_topoDataLastIndex branch from 9c7e1be to 3b628d5 Compare September 28, 2021 09:10
Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
@epernod epernod added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Sep 28, 2021
@epernod
Copy link
Copy Markdown
Contributor Author

epernod commented Sep 28, 2021

[ci-build][with-all-tests]

@hugtalbot hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Sep 29, 2021
Comment thread SofaKernel/modules/SofaBaseTopology/src/SofaBaseTopology/TopologyData.inl Outdated
…ogyData.inl

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
@fredroy fredroy added the pr: breaking Change possibly inducing a compilation error label Sep 29, 2021
@fredroy fredroy merged commit bca1afb into sofa-framework:master Sep 29, 2021
@guparan guparan added this to the v21.12 milestone Oct 1, 2021
damienmarchal pushed a commit to CRIStAL-PADR/sofa that referenced this pull request Oct 11, 2021
…a-framework#2359)

* [SofaBaseTopology] Update lastElementIndex to add m_  and add accessor. Generalize it's use in TopologyData during add or remove event to avoid bad access index during successive remove elements events.

(cherry picked from commit 133e76b)

* [SofaBaseTopology] Fix data last element should be only set by the right topological buffer this data is linked to. In case of cross topology linked, the size was overriden

* [SofaBaseTopology] Fix non windows compilation

* [SofaBaseTopology] Fix conversion warning

* Apply suggestions from code review

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>

* Update SofaKernel/modules/SofaBaseTopology/src/SofaBaseTopology/TopologyData.inl

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
@epernod epernod deleted the inf_fix_topoDataLastIndex branch March 14, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: breaking Change possibly inducing a compilation error pr: fix Fix a bug pr: status ready Approved a pull-request, ready to be squashed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants