Plugin Logic Proposal #75

kwokcb · 2024-06-17T20:34:23Z

Proposal

This is an initial proposed design for allow users to create compartmentalized plug-ins without having to override the main Qt application.

Design

Add in a simple QuiltiXPlugin class. For now this has the minimal of:
- An identifier string
- The plugin handler
Add in a simple QuiltiXPluginManager which is just a list of QuiltiXPlugin.
Access is provided to the editor as well as the package ROOT.
There are no constraints on what can and cannot be performed as is the case now when overriding the entire application.
It is up to the implementer to know the UI structure (also no change).

Discovery

A plugin is contained in a folder with a plugin.py file which contains a module with a install_plugin entry point.
One or more of these folders can reside under the following plugin "roots":
- The plugins folder under the root of the Python distribution.
- The folder specified by QUILTIX_PLUGIN_FOLDER

Example

I've migrated the JSON MaterialX logic to a materialxjson folder under a new sample_plugins folder.
Setting the env variable will allow this plug-in to be loaded.
The plug-in has read/write to JSON file and a "show as text" menu options.

@manuelkoester, @RichardFrangenberg : Let me know what you think of this. It's naturally very basic to start with.
If something like this is reasonable, I'd like to move on to adding glTF support this way. Thanks!

kwokcb · 2024-06-17T20:35:11Z

src/QuiltiX/qx_plugin.py

+logger = logging.getLogger(__name__)
+
+class QuiltiXPlugin():
+    def __init__(self):


More fields can be added. It's simple for now with a key and implementation.

kwokcb · 2024-06-17T20:35:37Z

src/QuiltiX/qx_plugin.py

+
+    def install_plugins(self):
+        self.plugins = []
+        plugin_roots = [os.path.join(self.root, "plugins"), os.getenv("QUILTIX_PLUGIN_FOLDER", "")]


This gives an initial package and env var location.

kwokcb · 2024-06-17T20:35:57Z

src/QuiltiX/qx_plugin.py

+                    spec.loader.exec_module(module)
+
+                    # Call module install_plugin function if it exists
+                    if hasattr(module, "install_plugin"):


A specific entry point is looked for.

kwokcb · 2024-06-17T20:37:20Z

sample_plugins/materialjson/plugin.py

+except ImportError:
+    logger.error("materialxjson.core not found")
+
+class QuiltiX_JSON_serializer():


There is no encapsulation of the core code as is the case with the current way of just overriding the entire application.

manuelkoester · 2024-07-08T20:22:21Z

Hey Bernard! Sorry for not getting to this earlier.
Thanks for this great PR. I would like to add some changes to it if you don't mind. It's mostly about making things a little bit more pythonic and maybe setting it up in a way where there could be hooks at multiple stages of startup, so plugins could decide when to run what code.
I hope to be able to get to it soon.

kwokcb · 2024-07-10T17:26:53Z

Hi @manuelkoester ,
Feel free to make whatever changes you want. I just wanted to put something up to see what you thought about opening this up. Just curious what stages of startup would be available ? Thanks for looking at this.

manuelkoester · 2024-07-14T17:44:16Z

Hey Bernard!

I've gone ahead and changed the plugin system quite a bit. It is now using pluggy in the backend, which also powers pytest. https://pluggy.readthedocs.io/en/stable/
This allows for multiple hooks, basically allowing us to hook into QuiltiX at different stages.
I've already went ahead and implemented some of these hooks.

I also changed the discovery a tiny bit, which allows for setting multiple paths in the QUILTIX_PLUGIN_FOLDER env var. For now I also removed looking into a "local" QuiltiX plugin dir.
Pluggy also makes it very simple defining new hooks, calling them in our code and for plugins to use them.

manuelkoester · 2024-07-14T17:45:38Z

src/QuiltiX/qx_plugin.py

+class QuiltixHookspecs:
+    @hookspec
+    def after_ui_init(self, editor: "quiltix.QuiltiXWindow"):
+        """
+        :param editor: The QuiltiX Window
+        """
+
+    @hookspec
+    def before_ui_init(self, editor: "quiltix.QuiltiXWindow"):
+        """
+        :param editor: The QuiltiX Window
+        """
+
+    @hookspec
+    def before_mx_import(self):
+        """
+        This allows any code to execute before MaterialX gets imported
+        """
+
+    @hookspec
+    def before_pxr_import(self):
+        """
+        This allows any code to execute before OpenUSD's pxr gets imported
+        """


These hookspecs specify the hooks, and what arguments the individual hooks get supplied.

Similar comment to the before_mx_import. Seems like it would be better to have as an after_pxr_import. Also maybe it's better to call this after_usd_import ?

Personally I prefer pxr, as AFAIK it is responsible for setting the env vars and startup. Also Usd at least in this context is just another submodule of pxr next to things like Gf, UsdGeom and the likes.

manuelkoester · 2024-07-14T17:46:32Z

src/QuiltiX/qx_plugin.py

+            if hasattr(module, PLUGIN_ID_FUNCTION_NAME):
+                plugin_id_function = getattr(module, PLUGIN_ID_FUNCTION_NAME)
+                plugin_id_name = plugin_id_function()
+                self.register(module, plugin_id_name)
+                logger.info(f"Registered plugin '{plugin_id_name} at {module.__file__}")


For now plugins are required to provide the plugin_id function in their plugin.py so it gets picked up by the Manager.

manuelkoester · 2024-07-14T17:47:07Z

src/QuiltiX/quiltix.py

+plugin_manager.load_plugins_from_environment_variable()
+plugin_manager.add_hookspecs(qx_plugin.QuiltixHookspecs)
+
+plugin_manager.hook.before_mx_import()


This is how the hooks actually get triggered.

I think it would be more useful to have an after MaterialX imported then the plugin can use the loaded version and if desired. Could cut down on overhead and also enhance consistent usage of the same runtime.

These named this way so users can theoretically still modify environment variables before MaterialX or the likes get initialized and read from them.
But yeah it would probably make sense to also add the after_ versions.

manuelkoester · 2024-07-14T17:51:06Z

sample_plugins/materialjson/plugin.py

+@qx_plugin.hookimpl
+def after_ui_init(editor: "quiltix.QuiltiXWindow"):
+    editor.json_serializer = QuiltiX_JSON_serializer(editor, constants.ROOT)


This is how and where we use the after_ui_init hook that get's triggered by the Manager.

RichardFrangenberg

Very nice and interesting approach. Feels very clean!

src/QuiltiX/qx_plugin.py

RichardFrangenberg · 2024-07-16T20:20:58Z

sample_plugins/materialjson/plugin.py

+try:
+    import materialxjson.core as jsoncore
+except ImportError:
+    logger.error("materialxjson.core not found")


This plugin isn't usable without jsoncore. Would it make sense to stop loading the plugin at this point and somehow disable it after logging the error?
Some logic for plugins to define requirements, which need to be met before the plugin can be loaded, could be handy. This could be used to define plugins, which only work on a specific OS or with a specific MaterialX versions.

Hi @RichardFrangenberg
There used to be a validity check before the refactor. I've added this back in with an explicit `is_valid()1 check on the plugin. It leaves it up to the plugin to decide where it can work.

I've changed it so a plugin doesn't need to implement is_valid, but it can choose to do so.

sample_plugins/materialjson/plugin.py

src/QuiltiX/qx_plugin.py

…(empty).

manuelkoester · 2024-07-17T16:02:23Z

Hey Bernard! I hope it is okay if I push over your changes with some of the things that I have started yesterday evening while chatting with Richard. It will functionally be very similar to yours however!

… plugins

manuelkoester · 2024-07-17T16:55:21Z

@kwokcb @RichardFrangenberg please feel free to leave any comments/feedback! Should be nearly done. The tiny test in tests\test_plugins.py also succeeds without any additional setup of env vars.

kwokcb · 2024-07-17T20:14:40Z

@manuelkoester Thanks for adding the pre and post. Looks good to me.
BTW, I created a 1.38.9 version of the dependent package if you want to test it out running. pip install materialxjson==1.38.9.

…upting a json load operation

manuelkoester · 2024-07-18T16:39:26Z

Nice! Works perfectly! I'll now merge this in. Thank you very much for the first implementation @kwokcb . There's very little we had to add here <3

qx plugin proposal.

40d5de2

kwokcb commented Jun 17, 2024

View reviewed changes

switched plugin system to pluggy to allow for mutliple hooks

ddfa194

manuelkoester reviewed Jul 14, 2024

View reviewed changes

manuelkoester requested a review from RichardFrangenberg July 14, 2024 17:56

massaged trigger connection for show_json

0874cd5

RichardFrangenberg reviewed Jul 16, 2024

View reviewed changes

kwokcb added 2 commits July 17, 2024 10:52

Add in is_valid() validity check and also check for invalid plug ids …

31d2e1d

…(empty).

Minor review fixes.

ae961e0

manuelkoester added 2 commits July 17, 2024 18:41

Changed validation method, added small test, added README section for…

bd94fd6

… plugins

fixed example in readme

b237609

manuelkoester requested a review from RichardFrangenberg July 17, 2024 16:50

Add in optional syntax hilighting if pygments is available.

33c8904

added pygments to plugin dependency, fixed small UX issue when interr…

ff279d3

…upting a json load operation

manuelkoester merged commit e34ebfe into PrismPipeline:main Jul 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin Logic Proposal #75

Plugin Logic Proposal #75

kwokcb commented Jun 17, 2024 •

edited

Loading

kwokcb Jun 17, 2024

kwokcb Jun 17, 2024

kwokcb Jun 17, 2024

kwokcb Jun 17, 2024

manuelkoester commented Jul 8, 2024

kwokcb commented Jul 10, 2024

manuelkoester commented Jul 14, 2024

manuelkoester Jul 14, 2024

kwokcb Jul 17, 2024

manuelkoester Jul 17, 2024 •

edited

Loading

manuelkoester Jul 14, 2024

manuelkoester Jul 14, 2024

kwokcb Jul 17, 2024

manuelkoester Jul 17, 2024

manuelkoester Jul 14, 2024

RichardFrangenberg left a comment

RichardFrangenberg Jul 16, 2024

kwokcb Jul 17, 2024

manuelkoester Jul 17, 2024

manuelkoester commented Jul 17, 2024

manuelkoester commented Jul 17, 2024

kwokcb commented Jul 17, 2024

manuelkoester commented Jul 18, 2024

Plugin Logic Proposal #75

Plugin Logic Proposal #75

Conversation

kwokcb commented Jun 17, 2024 • edited Loading

Proposal

Design

Discovery

Example

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuelkoester commented Jul 8, 2024

kwokcb commented Jul 10, 2024

manuelkoester commented Jul 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuelkoester Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardFrangenberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuelkoester commented Jul 17, 2024

manuelkoester commented Jul 17, 2024

kwokcb commented Jul 17, 2024

manuelkoester commented Jul 18, 2024

kwokcb commented Jun 17, 2024 •

edited

Loading

manuelkoester Jul 17, 2024 •

edited

Loading