Skip to content

Conversation

@eulertour
Copy link
Member

@eulertour eulertour commented May 22, 2021

Changelog / Overview

Motivation

Offers a way to get realtime feedback on variable parameters.

Explanation for Changes

Enables GUI interaction

Documentation Reference

Testing Status

Ran example GUI scene locally (Linux)

Further Comments

The API used for interacting with the GUI is subject to change, as right now it doesn't allow some things (like multiple windows)

Checklist

  • I have read the Contributing Guidelines
  • I have written a descriptive PR title (see top of PR template for examples)
  • I have written a changelog entry for the PR or deem it unnecessary
  • My new functions/classes either have a docstring or are private
  • My new functions/classes have tests added and (optional) examples in the docs
  • My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

  • The PR title is descriptive enough
  • The PR is labeled correctly
  • The changelog entry is completed if necessary
  • Newly added functions/classes either have a docstring or are private
  • Newly added functions/classes have tests added and (optional) examples in the docs
  • Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

@hydrobeam
Copy link
Member

hydrobeam commented May 22, 2021

Some things that I've noticed:

  • The gui initially starts at the bottom of the window and resets to this position everytime the scene is changed. It needs to be dragged up to actually be usable. It'd be nice if it started out in the center of the window and didn't need to be readjusted. Occurs in both windows and WSL.
    • image

@hydrobeam
Copy link
Member

hydrobeam commented May 22, 2021

  • Calling the gui test without specifying the scene from the command line: manim -p --renderer=opengl example_scenes/opengl.py causes the GUI interface not to appear. (the scene still plays)
    • Side note: it also asks for the scene twice (not caused by this PR)

@hydrobeam
Copy link
Member

hydrobeam commented May 22, 2021

  • Playing a GUI scene without the preview flag causes the GUI to appear and raises an error. Then after any input in the terminal, the program enters into a brutal never ending loop that forces me to kill my terminal. For the sake of avoiding this, the p flag should probably be automatically included when using the GUI.
    • input: manim --renderer=opengl example_scenes/opengl.py GuiTest
    • output: repeats indefinitely.
      Unhandled exception in event loop:
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\application\application.py", line 913, in inm    await _do_wait_for_enter("Press ENTER to continue...")
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\application\application.py", line 1282, in _r    await session.app.run_async()
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\application\application.py", line 816, in ruc    return await _run_async2()
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\application\application.py", line 792, in _r2    result = await _run_async()
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\application\application.py", line 721, in _rc    with self.input.raw_mode(), self.input.attach(
       File "C:\Users\laith\AppData\Local\Programs\Python\Python39\lib\contextlib.py", line 117, in __enter__
         return next(self.gen)
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\input\win32.py", line 594, in attach_win32_it    win32_handles.add_win32_handle(handle, callback)
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\input\win32.py", line 554, in add_win32_hande    run_in_executor_with_context(wait, loop=loop)
       File "C:\Users\laith\AppData\Local\pypoetry\Cache\virtualenvs\manim-zPbAr8g6-py3.9\lib\site-packages\prompt_toolkit\eventloop\utils.py", line 33, in run_in_exect    return loop.run_in_executor(None, ctx.run, func, *args)
       File "C:\Users\laith\AppData\Local\Programs\Python\Python39\lib\asyncio\base_events.py", line 814, in run_in_executor
         executor.submit(func, *args), loop=self)
       File "C:\Users\laith\AppData\Local\Programs\Python\Python39\lib\concurrent\futures\thread.py", line 161, in submit
         raise RuntimeError('cannot schedule new futures after shutdown')
      

Copy link
Member

@hydrobeam hydrobeam left a comment

Choose a reason for hiding this comment

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

Kind of botched the review feature but:

  • It'd be nice if the scene rendered in a fixed window at a certain position on the screen (like in manimgl). Right now, rendering a scene at full quality causes it to be off the side and difficult to manage.
    • Something like this would be ideal: image
    • The GUI can be placed just below the preview window, or next to it.
  • This is how it currently looks:
    • image
      • Note: I also had alt-tab to view these windows since they did not appear on top of the editor at first.

Copy link
Member

@hydrobeam hydrobeam left a comment

Choose a reason for hiding this comment

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

Some comments about the formatting of the GUI.

Copy link
Member

@Darylgolden Darylgolden left a comment

Choose a reason for hiding this comment

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

LGTM, given that this is experimental I think we can merge this now. This also fixes Intel rendering slowness despite @eulertour having no intention to fix it, and I would like to work on top of these changes to attempt to debug the Intel strokes problem.

Copy link
Member

@Darylgolden Darylgolden left a comment

Choose a reason for hiding this comment

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

Actually, why didn't the CI run for this PR?

@eulertour eulertour dismissed naveen521kk’s stale review June 13, 2021 01:36

Addressed in later commit

@eulertour eulertour dismissed Darylgolden’s stale review June 13, 2021 01:37

Addressed in later commit

@eulertour eulertour merged commit e326a7b into ManimCommunity:main Jun 13, 2021
@behackl behackl added the new feature Enhancement specifically adding a new feature (feature request should be used for issues instead) label Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature Enhancement specifically adding a new feature (feature request should be used for issues instead)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants