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.

Sign up for GitHub

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

Jump to bottom

[SofaGraphComponent] Run SceneChecker at each load #938

Merged
merged 10 commits into from
Mar 8, 2019
Merged

[SofaGraphComponent] Run SceneChecker at each load #938

merged 10 commits into from
Mar 8, 2019

Conversation

guparan
Copy link
Contributor

@guparan guparan commented Feb 18, 2019 •
edited by epernod
Loading

As discussed in latest dev meeting, we need SceneChecker output even at first load.

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.

@guparan guparan added enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request labels Feb 18, 2019
@guparan
Copy link
Contributor Author

guparan commented Feb 19, 2019

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

@guparan guparan 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 Feb 20, 2019
@guparan
Copy link
Contributor Author

guparan commented Feb 20, 2019

Will try to make it run with any GUI (currently only with RealGUI).

@guparan
[SofaGuiQt] Remove SceneChecker mechanism from GUI
08b8857
@guparan
[SofaSimulationCore] Delegate doLoad to force notifications
bae2f55 
SceneLoaders (derived from SceneLoaderFactory) must now override "doLoad" as they used to override "load".
-> Calling load will call notifyLoadingSceneBefore + doLoad + notifyLoadingSceneAfter

This permits to better customize pre/post scene loading with SceneLoader::Listener
@guparan
[SofaGraphComponent] ADD SceneCheckerListener for SceneCheck mechanism
18ef9d8 
SceneCheckerListener is a SceneLoader::Listener that will call SceneCheckVisitor::validate on every notifyLoadingSceneAfter event.
This permits to have SceneChecking working with any GUI (classic, batch) and any SceneLoader (XML, Python).
@guparan
[SofaGraphComponent] Better log for SceneChecker
ad2b46b
@guparan guparan 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 Feb 20, 2019
@guparan
Copy link
Contributor Author

guparan commented Feb 20, 2019 •
edited
Loading

So I think I found a clean way to run the SceneChecks by using SceneLoader::Listener.
SceneChecks are now run on every GUI (batch, classic) and every SceneLoader (XML, Python).
They are also triggered in unit tests that use loadFromMemory but that is maybe too much...

What do you guys think?

@guparan
Copy link
Contributor Author

guparan commented Feb 20, 2019

So apparently SceneLoaderPY (Python scene loader) needs to call notifyLoadingScene at a specific moment because using my new doLoad delegate messes with the unit test SofaPython_test/python/moduleReload.py

@damienmarchal Could you tell me if you see a fix to that?

We still can stick to the old load override and ignore doLoad in SceneLoaderPY.

@guparan guparan force-pushed the scenechecker_run_at_first_load branch from 7f4a595 to 229d05d Compare February 20, 2019 16:05
@damienmarchal
Copy link
Contributor

Actually the SceneListener was specifically designed for the python modules/scripts to be cleaned/reloaded when a new scene is loaded. This that few years ago so I forgot a bit how it work. But I'm sure it is important not to break it :)

@guparan guparan 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 Feb 27, 2019
@guparan
[SofaGraphComponent] MOVE SceneCheckerListener registration to runSofa
d3ed4d0 
+ add runSofa option to disable scene checking: noscenecheck
@guparan
[SofaPython] FIX SceneLoaderPY scene loading notifications
70d42ae 
loadSceneWithArguments did not call notifyLoadingSceneBefore and notifyLoadingSceneAfter
@guparan guparan 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 Mar 5, 2019
@guparan
Copy link
Contributor Author

guparan commented Mar 5, 2019

@damienmarchal I fixed the problem with SceneLoaderPY :-)

@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 Mar 6, 2019
@epernod epernod merged commit d12e1eb into sofa-framework:master Mar 8, 2019
@guparan guparan mentioned this pull request Apr 24, 2019
Closed
@guparan guparan added this to the v19.06 milestone Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement About a possible enhancement pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Milestone
v19.06
Development

Successfully merging this pull request may close these issues.
3 participants
@guparan @damienmarchal @epernod