Context menu: use of shared instances

[csaez]

Hi,

Today while re-working the integration of the outliner widget into our application framework (unfortunately we cannot use the dialogs as widgets need to get embedded) I noticed the assumption of singleton behaviour on some of the context menues launching dialogs, which is a bit problematic as we very often have multiple stages opened in the same session (via AL_USDMaya proxy shapes).

i.e.

usd-qt/pxr/usdQtEditors/outliner.py

Lines 135 to 142 in 35b01ea

	def Do(self, context):
	if context.selectedLayer:
	dialog = LayerTextEditorDialog.GetSharedInstance(
	context.selectedLayer,
	parent=context.qtParent or context.layerDialog)
	dialog.show()
	dialog.raise_()
	dialog.activateWindow()

I can certainly re-implement these MenuAction classes and the OutlinerRole on our side, but I feel like I'm already duplicating a big chunk of the glue code by not using the dialogs and was wondering if this use case would help to reconsider these default choices? Is there a particular reason for the reuse of the dialog?

Thanks!
C.

[nxkb]

Hi @csaez,
Thanks for making this issue. I've created #13 to deal more specifically with the pain of using the UIs as widgets. Let me know if that covers your needs there, and ill try to answer your other questions here:

The singleton allows us to ensure we don't have multiple text editors open for the same layer which could be confusing and result in lost work, especially if the changes are left unapplied. In practice this also helps you avoid opening a whole bunch of extra dialogs when making changes to the same layer. Since the layers themselves are reused between the stages... a singleton seems to be a good fit conceptually.

If you still think you need to have copies of a layer dialog, can you describe your use case a bit more?I'm thinking that even if layers are shared between proxyShapes, it still makes sense to have one dialog per layer. Any text editing changes should cause stage notifications that would ideally update the other outliners.

[csaez]

Thanks @nxkb,

Moving to widgets definitely helps! I reconstructed a bit of the project on our side in order to take advantage of the widgets (which wasn't that bad, I got to learn more of the project) but I imagine it would be desirable for other people using this to not have to do as much work to have a working embedable outliner running.

Regarding the singleton, it makes a lot of sense on that scenario, but it doesn't work too well when you deal with many stages in the same process.

Let me try to describe what I mean:

I have 2 outliners showing 2 stages within the same process, A and B, the first time I open the dialog showing a layer from A it will be created from scratch (as there's nothing cached) and correctly populated, it also will cache the dialog instance under the hood.

Let's say I want to inspect a layer from stage B now, if I run the same process this time the dialog will find a cached instance from the previous run, bypassing the init logic and showing the previous contents (from stage A), which is confusing and not at all what the user might expect.

What I want is not necessarily multiple dialogs, your explanation makes a lot of sense and I completely agree, I would like to make sure that the contents of the dialog correspond to the stage opened in the outliner where the context menu resides and not the one from the previous run.

Maybe adding a key/identifier to the cache is all it takes?!

Hope that helps,
Cheers!

[nrusch]

Hey @csaez, after talking this over with @nxkb, I think we're actually going to update the outliner to basically take "ownership" for all of the layer text editor dialogs it has created, which should sort this out. There should be a change coming shortly for this.

[nrusch]

Hey @csaez, we've pushed some new changes to the dev branch, which include the previously mentioned updates (at bee7c6d). Can you let us know if they would be sufficient to address this issue?

Just to give you a little more context, our rough plan is to do most of our rolling development on the dev branch, which we will be using internally in production, and periodically merge dev back into master after using it for a while without issues.

[csaez]

Thanks for the heads up, will give the dev branch a try and report back ASAP :)

Quick question: Would it be possible for additions to dev to go through pull requests? It would make changes to the project much more visible to the open source community.

Thanks!

[chadrik]

Quick question: Would it be possible for additions to dev to go through pull requests? It would make changes to the project much more visible to the open source community.

I would love to work that way, but the unfortunate reality is that internet access is restricted here due to Disney and MPAA security requirements, so we use an on-prem installation of gitlab to manage our internal pull requests. Doing all of our merge requests via github would add a lot of overhead to our developer's day-to-day workflows. I assume that you all at Animal Logic came to similar conclusion with AL_usdMaya, as you are following the same basic model. Same with Pixar and USD.

I think a good compromise solution could be for us to start creating proper release identifiers and change logs, with the caveat that we're still not at the point where we're willing to guarantee non-breaking changes, especially since we have to keep up with USD, which makes fairly frequent breaking changes which might manage to propagate up to the UsdQt APIs.

Another good step forward will be to get travis-ci setup so that we can test against an array of official USD releases. The big question there is: where can we download a prebuilt USD release? USD takes too long to build to include that as part of the testing.

Anyway, thanks for your contributions and your patience as we work towards a stable API.

[csaez]

Thanks Chad