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

[ALL] FIX all scenes failures 17.12 #565

Merged
merged 16 commits into from
Jan 24, 2018

Conversation

damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented Jan 18, 2018

This PR is a follow up #548
My last batch of changes to fix the CI. Please review :)


This PR:

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

Reviewers will merge only if all these checks are true.

Those file have been merge by mistake some time ago but they have no
meaning in the current Sofa version. So I remove them.
I know it is deprecated but...well it is better to update it so
this minimize the amount of work in case someone want to use it
in the future.
…verbose is true

Why:
serr is generating a lot of warnings which does not really make sense in this context.
In addition those 'useless' warnings flood the CI.
In this PR we:

- remove from the tests 'BezierTetrahedron.scn' as it depend on a plugin.HighOrder

- use msg_info instead of serr in BaseDeformationMapping
The component prints stuff that are really looking like info/debug
messages and I see ne reason why they should be error or warning.
In addition using 'sout/serr' is bad:
- it has an inconsistent naming as it emits warning (while users expect an error).
- it does not add the filename/line from where the message is emitted while msg_info does

So if, in a private fork, you want/need to continue to use the old 'serr' backend...please make your own macro
route msg_info to  the 'old' sout/serr backend instead of the one provided in Sofa.

- Fix examples/material/StVenantKirchoff.scn broken since the merge of In #PR 243 the following change was introduced in MechanicalObject.inl::init():
```cpp
  if (!l_topology && d_useTopology.getValue())
    {
        l_topology.set( this->getContext()->getActiveMeshTopology() );
    }
```

In place of
```cpp
    //Look at a topology associated to this instance of MechanicalObject by a tag
    this->getContext()->get(m_topology, this->getTags());
    // If no topology found, no association, then look to the nearest one
    if(m_topology == NULL)
        m_topology = this->getContext()->getMeshTopology();
```

I assume that the in the scene was the EdgeSetTopologyContainer is not returned by
the getActiveMeshTopology method.

NB: All those implicit linking looks really problematic to me as they are error prone.#PR 243
The plugin is broken since PR sofa-framework#549 as it removes the initialization
of the mutex. Possible explaination is that v_ is implementation
dependent but the existing initialization was done incorrectly
using a (int).

Now use BOOST_DETAIL_SPINLOCK_INIT which insure that the initialization
is coherent with the implementation used.

(I also cannot prevent myself to remove the two ugly commented lines)
The <include> directive was using local path instead of relative to the 'example'
directory as other examples.

NB: I wonder why the shared "Object" directory is in examples. If it is shared between different
examples scenes shouldn't it be in the 'share' directory ?
The color is encoded as a Vec4 not a Vec3
The points are now generated in the topology and can thus be used by the
mechanical model.

Note: I wonder how this was scene has ever worked. If any one knows please
tell, because my fix may not be the right way.
I remove the <EdgeSet /> because it seems to be a component that
does not exists anymore in Sofa.
@damienmarchal damienmarchal changed the title Fix scene failures1712 [ALL] FIX all scenes failures 17.12 Jan 18, 2018
@damienmarchal damienmarchal added pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request pr: fast merge Minor change that can be merged without waiting for the 7 review days labels Jan 18, 2018
@damienmarchal damienmarchal added this to the v17.12 milestone Jan 18, 2018
@guparan
Copy link
Contributor

guparan commented Jan 18, 2018

Thank you for this massive work Damien!
LGTM, waiting for CI to complete.

@damienmarchal
Copy link
Contributor Author

[ci-build][with-scene-tests]

@guparan
Copy link
Contributor

guparan commented Jan 19, 2018

My bad for Windows error, the dependency pack was updated too soon. #566 needs to be merged. In the meantime I restart your build with the old dependency pack ;-)

@damienmarchal
Copy link
Contributor Author

Before to merge ...wait the discussion with @epernod about cubetopology.

@guparan
Copy link
Contributor

guparan commented Jan 19, 2018

Ok. Discussion is in 47ef2e6

@guparan
Copy link
Contributor

guparan commented Jan 23, 2018

Any update on the discussion @damienmarchal @epernod ?

@guparan guparan 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 Jan 24, 2018
@epernod epernod self-requested a review January 24, 2018 09:28
@guparan guparan merged commit 52ac264 into sofa-framework:master Jan 24, 2018
@damienmarchal damienmarchal deleted the fixSceneFailures1712 branch February 12, 2018 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fast merge Minor change that can be merged without waiting for the 7 review days 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.

3 participants