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

Deferred Lua Execution #1180

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Deferred Lua Execution #1180

wants to merge 10 commits into from

Conversation

geringsj
Copy link
Contributor

@geringsj geringsj commented Apr 14, 2023

This PR restructures the Lua resources to provide a deferred lua execution method, such that services and the CLI can queue lua commands which get executed at one single, foreseeable point in the main loop.

Summary of Changes

  • add and use LuaScriptExecution and LuaScriptpaths resource structs
  • LuaScriptExecuton allows immediate and deferred execution mode
  • in main(), twice a frame the lua queue gets processed: before RenderNextFrame(), and inside RenderNextFrame() before rendering of entry points (after services possibly issued lua commands)
  • Project Loader, GUI, Remote Service and main() moved to use deferred lua
  • CLI inputs (project files, raw lua) can be executed in same order as passed via CLI

References and Context

This refactoring streamlines the Lua execution logic in the program, getting rid of project file and CLI Lua inputs confusion. This allows, for example, to load a project file and directly alter parameters of instantiated modules via CLI. At the same time, when taking control over the main loop via a lua project file, we remain outside of RenderNextFrame(). For the GUI, this allows for a strictly data-driven GUI re-design, decoupling of GUI event handling (before rendering of the frame) from rendering of the GUI draw list.

Challenges

// main render loop
while(run) {
execute_deferred_lua();
   {// RenderNextFrame()
      services.start_frame(); // e.g. resource update
      execute_deferred_lua();
      render();
      services.finish_frame();
   }
}

To keep the Lua execution outside of a frame, we need to flush the Lua queue before RenderNextFrame(). However, because services can issue Lua commands too (incoming network messages, keyboard/mouse + GUI inputs, resulting graph changes), Lua also needs to be flushed after service resource updates/digestion and before rendering of the graph. Having two Lua flushes leads to problems, e.g. Lua scripts coming from network get executed inside a frame, which is problematic when they contain mmRenderNextFrame(). However, not executing Lua after service updates ignores latest rendering state inputs for one frame, which seems even worse.

Test Instructions

CLI project files, param changes, lua execution, order of lua commands is respected: ./megamol.exe -p ../examples/testspheres.lua --param ::data::numSpheres=100 --param ::view::anim::play=true -e "print(\"\n\nHello MegaMol\n\n.\"); print(mmHelp())"

Load other project file after 500 frames: ./megamol.exe -p ../examples/testspheres.lua --param ::data::numSpheres=100 --param ::view::anim::play=true -e "for i = 1,500,1; do mmRenderNextFrame(); end; mmClearGraph()" -p ../examples/infovis-spheres.lua

2023-04-17-18-33-06_Trim.mp4

@geringsj geringsj requested a review from gralkapk April 17, 2023 09:39
@github-actions

This comment was marked as outdated.

… execution

this preserves order of CLI commands for lua execution. regrettably this also touches all parsing of CLI inputs, but should keep the logic sound.
@github-actions

This comment was marked as outdated.

1 similar comment
@github-actions

This comment was marked as outdated.

@reinago
Copy link
Member

reinago commented Apr 27, 2023

how much stuff has to actually run before services? could we get rid of the first execute_deferred_lua(); ? this feels a bit off tbh.

@geringsj
Copy link
Contributor Author

geringsj commented Apr 27, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants