Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GeneralTopology] Grid Topologies cleanup + new SphereGrid #164

Merged
merged 16 commits into from
Mar 8, 2017

Conversation

epernod
Copy link
Contributor

@epernod epernod commented Feb 9, 2017

PR on topologyGrid:

  • Add new component: SphereGridTopology to create sphere topo. using grid (similar to CylinderGridTopology) with an example: SphereGridTopology.scn
  • Clean the API GridTopology and its children: RegularGridTopology/SphereGridTopology/CylinderGridTopology, move several methods into mother class GridTopology.
  • Add doxygen doc for the 4 classes
  • Add tests for Cylinder and SphereGridTopology
  • Add method addSphere/addRigidSphere in sceneCreator + tests + example

This PR:

  • builds with SUCCESS for all platforms on the CI.
  • does not generate new warnings nor unit test failures.
  • does not break existing scenes.
  • does not break API compatibility.
  • has been reviewed and agreed to be transitional.
  • is more than 1 week old.
    Reviewers will merge only if all this checks are true.

…gy in init method in order to be able to set seqPoints values
…ints. Use only setSize to set the grid resolution. Nbr of points is computed implicitly.
…etPoint, getPx/y/z and getIndex from regular/cylindergrid to mother class GridTopology. Change name getPoint into getPointInGrid for inheritence mismatch. Only this last method should be overwritten by specific grid.
…hereGridTopology. FIX the alias factory for sphereGrid
…te the SphereGridtTopology.scn example to be more relevant.
…tor + tests on those methods and example in sceneCreatorBenchmarks
Conflicts:
	SofaKernel/modules/SofaBaseTopology/GridTopology.h
	SofaKernel/modules/SofaBaseTopology/RegularGridTopology.cpp
@epernod epernod changed the title Grid Topologies cleanup + new SphereGrid [GeneralTopology] Grid Topologies cleanup + new SphereGrid Feb 9, 2017
@epernod epernod added this to the v17.06 milestone Feb 10, 2017
@epernod epernod added the enhancement About a possible enhancement label Feb 10, 2017
@damienmarchal
Copy link
Contributor

Will be merged tomorrow unless someone send a no-go.

@damienmarchal
Copy link
Contributor

[ci-build]

@damienmarchal
Copy link
Contributor

It seems there is some compilation problems on this PR.
@epernod can you have a look ?

@hugtalbot
Copy link
Contributor

Hey @epernod ,

It seems that you PR is breaking on Windows VS 2015 : https://www.sofa-framework.org//dash/index.php?limit=10&pg=1&user=&branch=pr%2FSphereGridTopology&platform=

Could you have a look at it?
Cheers,

Hugo

@epernod
Copy link
Contributor Author

epernod commented Feb 27, 2017

Ok will check that by next dev-meeting.

@epernod
Copy link
Contributor Author

epernod commented Mar 6, 2017

Could someone give me a hint on what is the problem... @guparan or @damienmarchal maybe .
Looking for "error" in the full log give me this:
..\SofaKernel\framework\sofa\core\..\..\sofa/defaulttype/DataTypeInfo.h(757) : fatal error C1060: compiler is out of heap space c1xx : fatal error C1063: INTERNAL COMPILER ERROR

Not sure if this is really the error that breaks the compile and if it is related to my modifications or to the VM used for the build.

@damienmarchal
Copy link
Contributor

damienmarchal commented Mar 6, 2017

@epernod Sorry for that I'm quite sure this is not related to your PR.

EDIT: This week end we were upgrading all windows machine to faster ones. The problem you saw was specific to the memory consumption of vs2013 which, at certain moment larger than the allocated 12GB. This problem problem was solved by adding a larger virtual memory.

@matthieu-nesme matthieu-nesme merged commit 8410f29 into sofa-framework:master Mar 8, 2017
@epernod epernod deleted the SphereGridTopology branch May 4, 2019 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement About a possible enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants