pyblish / pyblish-qml

Pyblish QML frontend for Maya 2013+, Houdini 11+, Nuke 8+ and more
GNU Lesser General Public License v3.0
114 stars 44 forks source link

Fix #304, increase QML initialization waiting time #305

Closed davidlatwe closed 6 years ago

davidlatwe commented 6 years ago

Problem

In previous PR #304, the new feature that allow user to stop collecting process at startup, will make GUI stuck at "Collecting.." phase if the collecting process ended in short time (before QML get initialized).

That feature rely on QML to receive the signal firstRun to reset those button, so if the signal firstRun emitted before QML complete initialization, GUI stocked.

Fix

Increase QML initialization waiting time from 500 to 1500, give it enough time to be ready to receive signal firstRun

davidlatwe commented 6 years ago

With the increased waiting time, the state machine is now able to work right on the first reset, Footer.startup flag and other extra code are no longer needed.

davidlatwe commented 6 years ago

I just realized that there were some logic conflict in my description about the bug, I miss understood it. It does relate to video card's work load, but the cause of the bug wasn't quite right.

In PR #304, I added these code in pyblish_qml/qml/Overiew.qml here

        mode: {
            if (startup == true) {
                setMessage("Collecting..")
                return 1
            }
            return overview.state == "publishing" ? 1 : overview.state == "finished" ? 2 : 0
        }

This was a part of code for implementing stoppable collecting process. And this was the True Bug.

QML initialization waiting time not long enough only made the QML Footer not able to display stop button and "Collecting.." "Ready" message on startup, like this :

test

but won't block the process, so the fix code above just made it worse..

Therefore..

That feature rely on QML to receive the signal firstRun to reset those button, so if the signal firstRun emitted before QML complete initialization, GUI stocked.

This is not right, it didn't rely on that, but I make it does, and the bug pops.

Anyway, the bug isn't exists anymore, although increasing QML initialization waiting time wasn't the real medicine to fix (commit 786ca30 was the true fix), but did increase the chance to display GUI right.

If some how the wait-time value 1500 still not enough due to heavy background video card work load, the worst case was only not able to display stop button and "Collecting.." "Ready" message on startup.

Does this make sense to you ? Sorry again, for this boo-boo I made. :(

BigRoy commented 6 years ago

Thanks @davidlatwe - I'll play with this hopefully soon to test it. Just wondering, is the longer 1500 delay still needed if it's fixed already? What's the current worst case?

davidlatwe commented 6 years ago

Thanks @BigRoy :)

is the longer 1500 delay still needed if it's fixed already?

Not really needed, but would increase the chance that QML event handler get initialized to catch signal firstRun if background workload making it slow response.

What's the current worst case?

I think you meant 500 delay ? If so, then the worst case would be the first collecting process will startup with the GUI below, instead of "Collecting.." message and a stop button :

test

No messages and all the buttons were early exposed, not really serious.

BigRoy commented 6 years ago

Seems to be working fine on my machine here. Putting this live in production Today, if anything pops up I'll let you know.

davidlatwe commented 6 years ago

Thanks @BigRoy ! Great news !

BigRoy commented 6 years ago

It seems to work nicely here.

I did get this comment from one of the artists Today though:

I've had pyblish stall on repair actions btw, happened a couple of times today. Not sure why, but it just hangs on 'busy'

That would be the right click menu on plug-ins that trigger an action. I think it's unrelated to this particular feature (just thought I'd mention it here), but have you seen that happen too maybe?

davidlatwe commented 6 years ago

Hmm, No, I haven't. I do have some repair actions on hand, but didn't hanging in pass few day :)

davidlatwe commented 6 years ago

Yep, just checked the change again, should be unrelated, I could bet a case of beer :beer: , haha Thanks for approval @BigRoy : ) Then I will merge this tomorrow, if there's no other issue pops up.

BigRoy commented 6 years ago

Yep, just checked the change again, should be unrelated, I could bet a case of beer beer , haha

With how often artists are wrong I'd stay away from joining that bet. Nice work again David, thanks. :beers:

davidlatwe commented 6 years ago

Merging !