Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Pyblish Pype: Improve UI updates during processing #2646

Merged
merged 10 commits into from
Feb 12, 2022

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Feb 3, 2022

There was an issue with the recent speed ups for pyblish with UI not drawing correctly.

  • This now defaults to having "animations" disabled with OPENPYPE_PYBLISH_ANIMATED in settings. With the small timeout that was introduced recently with Publish pype: Reduce publish process defering #2464 which sped up the publishing by tons it did mean animations didn't really visually show anyway. So disabling them doesn't make much of a difference, except for that the UI correctly updates to the latest state.
  • There was some odd "on", "off', "on", "fade out" steps on the Log text bottom left of the UI. Not sure why, but that definitely didn't look good with the shorter intervals. Just having it "on", stay around for a while, and "fade out" looks much smoother and also avoids the text being drawn only half as if it's still updating the opacity. So I removed some animation steps there.

- This now defaults to having "animations" disabled - with the small timeout that was there '3' that didn't visually show anyway. So disabling them doesn't make much of a difference, except for that the UI correctly updates to the latest state.
@@ -74,6 +74,10 @@ def _execute(self):
item = self._items_to_process.popleft()
item.process()

# Process events directly after the item processed. This allows to
# correctly show "highlighted" state for the next item to process
QtWidgets.QApplication.instance().processEvents()
Copy link
Member

Choose a reason for hiding this comment

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

To be honest I and don't like this. It can trigger processing of next plugin directly or cause issues on different builds of Qt. If you'll change time interval back to 3 then you don't need to do it (interval 3ms == 0.003s).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you sure? Isn't that exactly what it is supposed to counter?

An interval or zero should wait for all events to process?. I'd be happy to change it back of course. Just wondering what could actually happen.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

I'd be happy to change it back of course. Just wondering what could actually happen.

The whole reason of using timer with timeout is to avoid calling processEvents. It caused issues on other than windows platforms especially in hosts with Qt binding.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted interval back to 3 with 6c12932

Copy link
Member

Choose a reason for hiding this comment

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

The goal was to remove this line with processEvents :)

Copy link
Collaborator Author

@BigRoy BigRoy Feb 4, 2022

Choose a reason for hiding this comment

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

Unfortunately it doesn't work as intended without that line.

See:
pyblish_pype_processEvents

Without processEvents() I had to put the timer_interval above 8 or so to sometimes get it to look right but it's visually slower to process (just due to the amount of plug-ins.)

Say there's about 60 plug-ins in Maya x 3 instances that can run into 60x3x8 = 1440ms vs 540ms. I mean those are not crazy differences if then "everything always works". But originally the delay was much higher, e.g. 50ms intervals would make it 60x3x50 = 9000ms = 9s that the UI would always add as overhead between running the plug-ins. And with 8 or 10 interval it still happens that some runs it doesn't actually highlight correctly, and I haven't had that happen with the processEvents() in my tests here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@iLLiCiTiT It also fixes the cases where if you press the STOP button during the Extract Pointcache that takes a long time that it actually stops after it's done. Otherwise it'd still continue to Integrate Asset New.

@iLLiCiTiT
Copy link
Member

Not sure why, but that definitely didn't look good with the shorter intervals

Removed comment in code because you're right. I think it had longer timeout in past.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 4, 2022

f13c83e Added a setting to disable the "info" printing to the console that just prints things like this:

Processing Collect Resources Path
Processing Validate Containers
Processing Validate Version
Processing Validate Version
Processing Instance Nodes Have ID
Processing Maya Workspace Set
Processing Node Ids in Database
Processing Node Ids in Database
Processing Subset Name
Processing Subset Name
Processing Resources Unique

Note that the duplicate ones are "per instance"

However, I kept the default setting to have it enabled so you won't notice unless you set the env var or explicitly override the setting in settings. Printing in Maya tends to be slow - so disabling this basically meant we really started to break the space-time continuum. I did it more because it clugged the Script Editor and made debugging much harder for me when looking at the console.

@BigRoy BigRoy requested a review from iLLiCiTiT February 4, 2022 08:45
@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 12, 2022

Thanks to some tweaks by @iLLiCiTiT the explicit call to processEvents() is now removed but the UI updates the same. This works well on my end - tested in Maya.

@iLLiCiTiT iLLiCiTiT merged commit 3d1ad12 into ynput:develop Feb 12, 2022
@BigRoy BigRoy deleted the pyblish_pype_update branch March 20, 2024 15:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants