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

Add render settings widget #18

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

pablode
Copy link
Contributor

@pablode pablode commented Aug 6, 2023

This PR adds a widget that allows the user to interact with the Hydra render delegate settings. See it in action here:

2023-08-06.16-58-33.mp4

@manuelkoester
Copy link
Member

This is awesome Pablo, thank you for your contribution!

I think we will probably need a vertical scroll to enable renderers with a lot of render settings. Your looks great, I'll add some comments here and there

Copy link
Member

@manuelkoester manuelkoester left a comment

Choose a reason for hiding this comment

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

This is great! It would be increadible if you could address these 2 comments. Otherwise we can also merge and add some additional work on our end.

Awesome contribution ❤️

@@ -209,9 +226,6 @@ def on_node_graph_changed(self, nodegraph):
def get_stage_tree_widget(self):
return usd_stage_tree.UsdStageTreeWidget()

def get_stage_view_widget(self):
return usd_stage_view.StageViewWidget()

Copy link
Member

Choose a reason for hiding this comment

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

We use these functions to act as a way of dependency injection.
This way our constructor doesn't need a ton of parameters. If one wants to us their own StageViewWidget they can overwrite it via (pseudocode):

class MyStageView(StageViewWidget):
    pass


class MyQuiltiX(QuiltiX):
    def get_stage_view_widget(self):
            return MyStageViewWidget()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense. I was wondering what they were for. I've reintroduced them in the latest commit.

super(RenderSettingsWidget, self).__init__()
self.setAttribute(Qt.WA_StyledBackground, True)
self.stage_view = stage_view
self.layout = QFormLayout(self)
Copy link
Member

Choose a reason for hiding this comment

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

Here I think we would need a QScrollArea() around the QFormLayout to deal with renderers which have a lot of render settings.

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 have implemented the suggestion as follows:

--- a/src/QuiltiX/usd_render_settings.py
+++ b/src/QuiltiX/usd_render_settings.py
@@ -1,15 +1,21 @@
 from Qt.QtCore import Qt # type: ignore
-from Qt.QtWidgets import QDoubleSpinBox, QLineEdit, QFormLayout, QWidget, QCheckBox, QSpinBox  # type: ignore
+from Qt.QtWidgets import QDoubleSpinBox, QLineEdit, QFormLayout, QWidget, QCheckBox, QSpinBox, QScrollArea  # type: ignore

 from pxr.Usdviewq.stageView import StageView, UsdImagingGL

-class RenderSettingsWidget(QWidget):
+class RenderSettingsWidget(QScrollArea):

     def __init__(self, stage_view, window_title="Render Settings"):
         super(RenderSettingsWidget, self).__init__()
         self.setAttribute(Qt.WA_StyledBackground, True)
         self.stage_view = stage_view
-        self.layout = QFormLayout(self)
+
+        innerWidget = QWidget(self)
+        self.layout = QFormLayout(innerWidget)
+        self.setWidget(innerWidget)
+        self.setWidgetResizable(True)
+        self.setHorizontalScrollBarPolicy(Qt.ScrollBarPolicy.ScrollBarAlwaysOff)
+        self.setVerticalScrollBarPolicy(Qt.ScrollBarPolicy.ScrollBarAsNeeded)

     def on_renderer_changed(self):
         self._clear_widgets()

however, I'm not quite sure about

  1. the initial dock widget size
  2. horizontal sizing/scroll behavior
  3. styling (padding/margin) with respect to the node properties window.

Perhaps it would be best to merge and you can resolve these points internally?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds great to me!

@manuelkoester
Copy link
Member

Thanks again Pablo, will merge this soon :)

@manuelkoester manuelkoester merged commit 381d3e1 into PrismPipeline:main Aug 15, 2023
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.

None yet

2 participants