BigRoy / usd-qtpy

Python Qt components for building custom USD tools.
MIT License
59 stars 9 forks source link

Added QMenuBar to EditorWindow, with dummy submenu items for now. #19

Closed Sasbom closed 10 months ago

Sasbom commented 10 months ago

Adds a theme conformant menu bar to the top of the editor window. As simple as it gets!

Sasbom commented 10 months ago

I should really go download a stub file for qtpy, I completely missed the return type of the addMenu function! I implemented the changes and cleaned some remaining mess up.

BigRoy commented 10 months ago

Learn something new every day - didn't know about layout.setMenuBar thanks!

The question really becomes, for the main Editor window; what kind of menu entries are we expecting to exist.

File > Open
File > Reload
File > Save
File > Save As

Maybe some options to hide/show the individual widget panels:

Panels > Layer Tree
Panels > Prim Hierarchy (Outliner)
Panels > Viewer
Panels > Layer Prim Spec Editor

We could have an About or Documentations menu entry and Help but it'd make it quite specific to the lib which kind of goes against the "reusable widgets" and also there's hardly enough "stable" yet to build that as well.

Sasbom commented 10 months ago

Learn something new every day - didn't know about layout.setMenuBar thanks! No problem, learnt about it today too ;)

The question really becomes, for the main Editor window; what kind of menu entries are we expecting to exist.

File > Open
File > Reload
File > Save
File > Save As

I feel like this is a hard one to call. If the window is opened as a "viewer" one might not want to be able to load a different file through the interface, because that'd be handled externally. Saving options could be a

File > Save
File > Save as
File > Save All Layer Changes

With the last option being there to not have to go through all layers to make a change. Thinking about it, a natural question that pops into my head is, that there might be some need for a "save project" approach as well, where the working state of the app could be saved for continued work some other time, but i feel like that goes beyond the scope of this tool, and it's not the intended purpose.

Maybe some options to hide/show the individual widget panels:

Panels > Layer Tree
Panels > Prim Hierarchy (Outliner)
Panels > Viewer
Panels > Layer Prim Spec Editor

This is exactly that I was thinking of, that'd be first on the list of stuff to implement in a menu.

We could have an About or Documentations menu entry and Help but it'd make it quite specific to the lib which kind of goes against the "reusable widgets" and also there's hardly enough "stable" yet to build that as well.

Included docs I feel aren't nessecary, except for maybe a link to repo-hosted documentation on the viewer itself. I agree that there currently isn't that much stability to go off of yet.

I feel like the menu should host the "higher level" tools as well, like a turn table feature, selecting a hydra delegate (if more than the standard GL renderer is found), things like that.

Render > Still
Render > Preview Render (Playblast)
Render > Turntable > ... (different turns to be tabled)

The Still Render option would just allow a render with a custom resolution, essentially 1 frame of Playblast.

BigRoy commented 10 months ago

Sounds good - let's see if we can get in the panel toggles first.

Should be as trivial as storing a reference to the widgets on the Editor Window and then likely using widget.setHidden(not widget.isHidden())

Sasbom commented 10 months ago

Sounds good - let's see if we can get in the panel toggles first.

Sounds like a plan! I will see if I can reflect the checked state in the submenu items with an icon something similar. If I don't look at it tonight I will give it a shake this weekend.

Sasbom commented 10 months ago

After doing some reading, I'm going to attempt doing it Qt's way first, which would be neater in the end I think.

https://wiki.qt.io/Qt_for_Python_Signals_and_Slots

That'd mean implementing QtCore.Slot methods to connect to a signal emitted by the menu action QWidget already derives from QObject so implementing those would be trivial. That way, we can emit a bool directly and handling it in the widget itself, instead of relying on a function that emulates this functionality.

EDIT: I'm not too sure about this now that I look at it closer. I will see if I end up going this route.

BigRoy commented 10 months ago

After doing some reading, I'm going to attempt doing it Qt's way first, which would be neater in the end I think.

https://wiki.qt.io/Qt_for_Python_Signals_and_Slots

That'd mean implementing QtCore.Slot methods to connect to a signal emitted by the menu action QWidget already derives from QObject so implementing those would be trivial. That way, we can emit a bool directly and handling it in the widget itself, instead of relying on a function that emulates this functionality.

EDIT: I'm not too sure about this now that I look at it closer. I will see if I end up going this route.

You don't need to have slots for this I think.

You can just action.toggled.connect(widget.setVisible) - indeed without the need for additional functions. However, if for whatever reason the UI would still get hidden (not from the action) I believe the check state of the QAction itself would still get de-synced. I'm not sure if there's any built-in way to sync those up.

Asking on Tech-Artists slack there's no signal emitted by default from a QWidget as it hides or becomes visible - so there's nothing you can 'connect back' to the QAction to link it's checked state. However you could subclass widgets and implement showEvent and hideEvent to emit your own signals to do so - but I'd say it's a bit overkill just for ensuring a sync state of the checkbox only for edge cases. Let's for now keep things simple and just assume the actions will be defining the state of the panels?

I'll update the above example to just use that direct connection from signal to the widget.setVisible method -> updated

Sasbom commented 10 months ago

but I'd say it's a bit overkill just for ensuring a sync state of the checkbox only for edge cases. Let's for now keep things simple and just assume the actions will be defining the state of the panels?

Sounds good, i got lost in curiosity.

However you could subclass widgets and implement showEvent and hideEvent to emit your own signals to do so

The way I was planning to do it was through something I really REALLY do not like, which is multiple inheritance. I haven't written any code for this yet, I've just been playing around with the signal system, but it was going to end up being something like this:

class enableHideSignal(QtCore.Widget):
    onChangeVisibility = QtCore.Signal(bool)

    def setVisible(self, visible : bool):
       super(enableHideSignal,self).setVisible(bool)
       self.emit.onChangeVisibility(visible)

then, later in the widgets, inherit.

class EditorWindow(QtWidgets.QDialog, enableHideSignal):
    ...

and catch it with some

@QtCore.Slot(bool)
def update_visibility(visibility):
    ...

that would keep the state of the menus in check.

However, I don't know if the __mro__ would play nice with this, along with getting stuck on how to neatly pass along the right object, so i feel like your approach is better.

I feel like to sync the state of the menus, we could probably add a hover catcher for the menus that'd need it. That way, we can update as soon as the mouse enters the containing "Panels" menu.

BigRoy commented 10 months ago

You are right. I should be more awake today.

That is what menu.aboutToShow signal can be used for to update the check states of the children actions for this particular menu and we are good to go.

Let's avoid the multiple inheritance. Especially PySide6 doesn't like that. ;)

Sasbom commented 10 months ago

That is what menu.aboutToShow signal can be used for to update the check states of the children actions for this particular menu and we are good to go.

Alright! I'll implement it into the next pr. It's really funny to see how you can just abuse the data property to pass stuff around. I've never seen that before!

Let's avoid the multiple inheritance. Especially PySide6 doesn't like that. ;)

Yes O _ O by all means Don't worry, I usually don't even use inheritance at all. This was one situation where dark forces were tempting me....

BigRoy commented 10 months ago

Thanks for the contribution. I tweaked it slightly, will merge it now.

Interesting - somehow my push to this editable PR isn't showing on Github. Ah, apparently it is "still processing": image

Never seen that before.