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

Fusion - Revamp OpenPype menu #5416

Conversation

EmberLightVFX
Copy link
Collaborator

@EmberLightVFX EmberLightVFX commented Aug 6, 2023

Changelog Description

This PR revamps the OP menu inside of Fusion. Instead of having to load an OP instance in Fusion and using a custom UI window we now communicate with OP though a socket. We send the Fusion UUID together with what menu command to execute leading to the execution happening in the correct Fusion window if the user have multiple Fusion instances open.

Additional info

  • Everything should work as expected but I haven't currently bin able to open any UI yet. When I do I get the error openpype.pipeline.anatomy.ProjectNotSet: Implementation bug: Project name is not set. Anatomy requires to load data for specific project. I'm sure I'm executing the command to open any window the wrong way. I have tried to look how other addons does it but I never found a solution. Would love some help with this part!
  • I also left the old OP Menu UI in the Fusion menu so I could see if I broke something else while trynig to get everything working.
  • Currently the socket port is a hardcoded 12345 but will be made into a setting later on.
  • The path to the fusionscript dll is hardcoded. It should be fetched from the entered fusion path from the settings.

Todo

  • Create a new process for each Fusion launch
  • Make socket port dynamic and passed to Fusion as a env var
  • Clean up old code and remove unused files
  • Make fusionscript path dynamic to the fusion location
  • Add keyboard shortcuts to the menu items

Testing notes:

  1. Open an asset in Fusion
  2. Use the OpenPype menu to open any window

@ynbot ynbot added size/S Denotes a PR changes 100-499 lines, ignoring general files type: enhancement Enhancements to existing functionality host: Fusion labels Aug 6, 2023
Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

Nice prototype. I didn't test it yet. But did have some questions/comments, so I added them to the review.

I commented on the forum about why likely the tools are not working - it's likely due to install_host() not having been triggered in the socket listening process (where all the tools are actually running) but it seems that you are triggering install_host(FusionHost()) - then the only thing I can still think of is that the socket listener process isn't or doesn't stay completely in sync with the Fusion session. It looks like you're running the listening process straight inside the launcher Python process. I'm quite confident that won't work. You'll at least need to spawn of a custom python process that is NOT the tray/launcher process but dedicated to the fusion process solely for the purpose of being dedicated to the right environment (os.environ tools, but also openpype context like project, asset, etc.) since I believe there currently isn't a way in OpenPype to run multiple OpenPype context including tools together within a single python process.

openpype/hosts/fusion/api/lib.py Outdated Show resolved Hide resolved
data_json = json.dumps(data_to_send)

client_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server_address = ("localhost", 12345)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm pretty sure this won't work for multiple fusion sessions open at the same time. They'd both be sending the commands to the same process - and thus, that process that should be "in a certain openpype context, like project, asset, shot, etc." now suddenly has to be in two contexts at the same time. I feel like this will not work and we'll need to have at least a separate process per fusion session.

Also, sessions can crash. I feel there'll be a need to IF no response is received that maybe we'll restart the 'menu' process for the current session.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds logical. I was thinking of passing the socket to Fusion over an environment variable. That way it could be randomly picked so it won't already be in use and simply pass it like any other env var.

In short there would be a pre-hook that executes exactly like the old Menu UI object but instead of creating the UI it creates a socket listener and we control the process through socket commands.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I will need to get some help on how to set up the new process correctly as I have no idea how to do that in OP.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Best would be to have one process tho and pass what project and asset is being used. That way we could get tabs in Fusion to work correctly! But that could be done in a later PR

Comment on lines 22 to 28
def execute(self):
# Connect the triggered signal to the internal function's execute slot

menu_socket_listener = MenuSocketListener()
menu_socket_listener.start()

self.log.info("Starting socket listener")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could set an env var here like OPENPYPE_FUSION_SOCKET_PORT and make it unique per Fusion session in a way that if a socket is already in use by another fusion session then we'll need to pick another port for this particular fusion session.

@iLLiCiTiT
Copy link
Member

I just quickly scrolled through the code, but if I understand the implementation correctly, it is one-way communication, which is alive only few miliseconds (send message and forget). Each tool has it's own process -> technically there can be multiple processes with the same tool (have opened workfiles tool 0-n times). I'm not sure how "current context" would be propagated to/from the fusion process, e.g. on context change with workfiles tool.

@EmberLightVFX
Copy link
Collaborator Author

@iLLiCiTiT a new process would need to be started each time the user opens a comp. Fusion then communicate with that process.
Currently I'm using the traybar-process from what I understand. I would need to get a new process to start like I described above.
The python script from Fusion is only for communication.

I'm away right now but might be able to rethink some parts and see how it would be possible to implement this correct

@EmberLightVFX
Copy link
Collaborator Author

So I have gotten this PR working!
When you launch Fusion a new process gets launched that Fusion will communicate with.
Some problems:

  • It seems that the old publisher gets loaded instead of the new one
  • When you open a menu item a new QApplication gets created. When you close the menu item and re-open it it tries to use the old QApplication but the window becomes white and freezes.

I'm starting to question if all this work is worth it or not 😅
It's nice to have the terminal outside of Fusions console as OP fills it up with error messages (even if it's not error messages) but yeah... feels like quite some coding left to make it work flawlessly

@mkolar
Copy link
Member

mkolar commented Feb 6, 2024

Because we're splitting OpenPype into ayon-core and individual host addons, this PR would have to be re-created to target one of those.

I'm closing it down, but we'll he happy for a new PR to ynput/ayon-fusion repository once it's up.

@mkolar mkolar closed this Feb 6, 2024
@ynbot ynbot added this to the next-patch milestone Feb 6, 2024
@jakubjezek001 jakubjezek001 removed this from the next-patch milestone Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community contribution host: Fusion size/S Denotes a PR changes 100-499 lines, ignoring general files type: enhancement Enhancements to existing functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants