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

Batch renderer integration into Python viewer. #1964

Merged
merged 96 commits into from
Dec 22, 2022

Conversation

0mdc
Copy link
Contributor

@0mdc 0mdc commented Dec 8, 2022

Motivation and Context

This changeset integrates the batch renderer into the Python viewer.

New command line args:
--enable-batch-renderer enables batch rendering.
--num-environments specifies the number of concurrent environments to render.
--width, --height specify the window resolution.
--composite-files allows to pass composite gltf files to Magnum.

Example usage:
python examples/viewer.py --enable-batch-renderer --num-environments 6 --width 1024 --height 896 --scene "data/scene_datasets/habitat-test-scenes/skokloster-castle.glb"

batch_renderer_6env

python examples/viewer.py --enable-batch-renderer --num-environments 64 --width 1024 --height 1024

batch_renderer_64env

How Has This Been Tested

This was tested locally, and in automated tests here: #1947.
No regression found when testing the viewer with the classical renderer.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

0mdc and others added 29 commits November 24, 2022 10:23
As there's already an API to change the transformation of a hierarchy
right after it's added, this parameter was kinda redundant.

But I need to have a way to set an initial "coordinate frame correction"
transformation and keep it there, forever, which is what this parameter
could do instead -- so now it applies the transform to the immediate
child nodes instead, leaving the top-level transformation as an identity.
Need this to achieve a pixel-perfect correspondence with the classic
renderer.
It was in the CUDA interop test already but that one isn't ran on
non-CUDA builds.
I don't intend these to be used that much, nevertheless it's good to not
do stupidly inefficient things such as allocating and then copying a
huge buffer every frame just because "it's fine for a test".

It also allows for easy querying of image subranges, which is a nice
bonus.
Generalizes blitRgbaToDefault(), which was a bit too limited.
…rge it cleanly.

Because there's a thing called clang-format which makes any clean merges
just straight impossible. Yay!
The public base class interface is now non-virtual, which allows it to
perform various checks before delegating into the concrete
implementation. Which allows the implementations to omit all checks that
would otherwise be duplicated between them.
Also putting that back to a single file -- a Player can't really be
used without the implementation class anyway so a separate header
doesn't make sense.

The light setup and semantic ID setting is optional, with the default
implementation being a no-op.

Also changed gfx::replay::GfxReplayNode* to gfx::replay::NodeHandle.
It's shorter and the pointer is a part of the typedef, thus removing a
potential ambiguity about whether one should use a * with this type or
not.
@0mdc 0mdc requested a review from mosra December 8, 2022 19:30
@0mdc 0mdc marked this pull request as ready for review December 19, 2022 19:43
@0mdc 0mdc changed the title [WIP] Batch renderer integration into Python viewer. Batch renderer integration into Python viewer. Dec 19, 2022
CORRADE_ASSERT(
!Magnum::GL::Context::hasCurrent(),
"Simulator::reconfigure() : Unexpected existing context when "
"createRenderer==false", );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this assert as it is obsolete.
In this case, a context exists (GLFW) but the simulators are headless.

Copy link
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

Good progress here. I left some comments.

examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
examples/viewer.py Outdated Show resolved Hide resolved
src/esp/bindings/SimBindings.cpp Show resolved Hide resolved
src/esp/sim/SimulatorConfiguration.h Outdated Show resolved Hide resolved
@0mdc 0mdc force-pushed the python-viewer-batch-renderer branch from 928a19f to 7724832 Compare December 21, 2022 22:21
@0mdc 0mdc merged commit ff85ab9 into facebookresearch:main Dec 22, 2022
@0mdc 0mdc deleted the python-viewer-batch-renderer branch April 1, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants