WIL*: New tutorial system, better child window management

[ferdnyc]

* WIL == "Work In Limbo"

At the moment the status of this is "on hold indefinitely", until I can revisit its status more concretely, and figure out a plan for revisiting the PR itself.

Previous Update

To my very great surprise, to be perfectly honest, initial tests of this code in Win7 (after development on Linux) seemed to indicate that it requires no changes to function equally well on that platform. This is something I was genuinely not expecting, and apparently means that the code may be more finished than I thought. So, in honor of that surprise development, I'm creating this PR to open the code up to wider review and testing.

The set of commits in this PR do two basic things:

1. Re-work the OpenShot main window's communication with its child windows
2. Completely restructure the way tutorial popups are created and managed

You can see the individual commits for further details, but the main takeaways are:

1. There is a new Tutorial dock widget defined in main-window.ui, initially hidden and permanently floating and non-dockable.
2. All Tutorial popups are now displayed inside of that (same) dock widget, which has all of its decoration removed so that it is completely invisible (aside from the tutorial popup itself).
3. Any move/resize events on the main window are communicated to the Tutorial dock, and it updates its position accordingly (just as before), but because it's a QDockWidget, Qt makes sure it stays on top of the main window. Also, because it's a dock, not a window, it doesn't appear in the window list.
4. Show/hide events from the main window are now propagated to ALL floating dock widgets (not just the tutorial, though this does apply to the tutorial as well). Which means that if you minimize OpenShot, floating docks will minimize with it, and they'll return when you restore it. No more "the tutorial disappeared when I minimized OpenShot, but Properties didn't, and when I restored OpenShot the tutorial was gone" type issues.
5. The expensive event filters on both the main window and the tutorial widget are gone, as they're no longer needed.
6. Calls to show() on both the main window and the tutorial widget are now tightly controlled to avoid unnecessary window updates and repaints, which I am very hopeful will alleviate the flickering and unresponsiveness seen by some users (see Tutorial popups have nomal window type #1080, tutorial window flickers at opening #1264, Tutorial Dialog Flashing #1555, Tutorial will not go away. #1623, etc.)

There are some unresolved issues/questions, some of them stemming from this change, some not.

• Of the ones not directly a consequence of this change, the most pressing is the question of what to do about floating docks when OpenShot it maximized/fullscreened. It's really not meant to have docks floating on top of the main interface, and in practice it doesn't work well (tutorial popups excepted). I suspect it might be a good idea to auto-dock any floating panels on maximize, and then automatically float them again when the window is returned to normal state. (The tutorial popups can't be docked, so they wouldn't be affected.)

• In terms of the tutorial the main problem is, while Qt makes sure it always stays on top of the main window, I'm having less success getting it to always stay on top of other floating dock windows, especially after a minimize/restore cycle. So it's a bit annoying when the tutorial is pointing to one of the dock panels, you minimize the application, and then when you restore it suddenly the tutorial is behind the floating dock. But I don't want to get too aggressive with excessively show()-ing and raise_()-ing things to try and work around that, as that's where we get into trouble with too-frequent updates. A solution that ends up recreating the previous situation with repaint spam, flickering, and windows that won't take clicks is no solution at all.

  Also, because the tutorial popups are inside a QDockWidget that changes size, technically they may be resizeable on some systems. (They're not on Windows, they are in Gnome Shell on Linux if you can find the tiny, invisible hairline border.) But even then, the tutorial resizes just fine, and it'll be automatically resized again when advancing to the next popup, so I really don't consider it a problem if the user can resize them.

I need to explore a range of different scenarios, including switching between different window states and between Simple and Advanced view, and test those scenarios on multiple systems to make sure that the tutorials don't do anything crazy at some point. But I wanted to at least open the code up to review and comment in its current state, and of course if anyone has a chance to test it and report the results then that would be a huge help.

[peanutbutterandcrackers]

@ferdnyc - It hit me the other day: how do other softwares that use PyQT5 do pop-ups (if any of them do)? Perhaps that could also provide you with a nice insight as to how to deal with things...?

[ferdnyc]

I don't think there is any general way it's done. (Honestly, PyQt is not widely-used enough to really offer a large selection of other examples. Qt itself is far more widespread. But popup widgets are a fairly uncommon design pattern to begin with. I'd have to find another Qt application that uses popup widgets at all, offhand I can't name any.)

I mentioned in one of the commit messages that using a QDockWidget for transient child windows was suggested in a Stack Overflow answer (in a different context and not involving PyQt), that's how I got the idea in the first place. Immediately I realized that it would solve all the problems of keeping the widget above the main window, which was reason enough to go that route. I still had to figure out how to make the window decoration invisible, which was no small feat, but once I figured that out and realized I only had to do it once if I reused the same window, everything fell into place.

[peanutbutterandcrackers]

Hmm... I see. Looking forward to further commits in this PR. And I really really hope that someday you would make further enhancements (intelligent pointer) to the tutorial pop-ups too. That'd be pretty neat.

Awesome work! 👍

Perhaps @DylanC and @N3WWN would also have some input regarding this PR?

[ferdnyc]

Hmm... I see. Looking forward to further commits in this PR.

I don't know that there will be any... I'm hoping not, in fact. With any luck it's pretty much good to go as-is. Right now what it most needs is testing. If my own testing fails to show up any major problems, and nobody else reports encountering any, I'll probably remove the WIP flag and see if we can get it into some daily builds so it can be tested by the users who've reported issues with the old code.

The only outstanding concern that I've found so far with the new code is still the thing where tutorial popup doesn't always restore on top of the floating dock it's pointing to, after OpenShot is minimized — and that's such a corner case that I'm reluctant to really even try addressing it (for fear of destabilizing the code). "Don't minimize OpenShot if you have a floating dock with a tutorial popup pointing to it" doesn't seem especially limiting to the users.

(The odds of a user having floated any of the dock panels before running the tutorial seems extremely slim to begin with, in the organic case of someone who's encountering the tutorial for the first time, rather than re-running it for testing purposes.)

Beyond that, I'll address any issues that turn up in my or anyone else's testing, but... the code working just fine on Windows already surprised me, maybe it'll continue to surprise me.

[peanutbutterandcrackers]

Alright then. I will try to arrange for something to get this tested with. Perhaps a 'testing' branch in the repo itself. But I'm going to have to talk to Mr. Thomas first. I don't want to destroy the build system. :D

[peanutbutterandcrackers]

Hey there! Just created a new branch that incorporates these changes: test/new_tutorial_system, and it should build soon. Then we can open this up for wider testing. 👍

Edit: For some reason, it didn't actually build but treated the new branch as just another PR. So, we'll have to wait... :/

[ferdnyc]

Just pushed a change to prevent this sort of thing (with the "Humanity" theme), so that'll also need to be incorporated into the branch.

@peanutbutterandcrackers :

For some reason, it didn't actually build but treated the new branch as just another PR.

Well, all PRs are effectively branches — the CI system even reports, right below, "This branch has no conflicts with the base branch". Assuming it's even building, though, I think the issue may be that installer/build-server.py includes (lines 245-267):

        if git_branch_name == "develop":
          # ...
        elif git_branch_name == "release":
          # ...
        elif git_branch_name == "master":
          # ...
        else:
            # Make filename more descriptive for daily builds
            openshot_qt_git_desc = "%s-%s-%s" % (openshot_qt_git_desc, version_info.get('libopenshot').get('CI_COMMIT_SHA')[:8], version_info.get('libopenshot-audio').get('CI_COMMIT_SHA')[:8])
            # Get daily git_release object
            github_release = get_release(repo, "daily")
            needs_upload = False

AFAICT that needs_upload = False prevents exporting any builds not generated from one of the primary three branches.

[peanutbutterandcrackers]

Sorry... still have to deal with some other things. Technical difficulties.

But I am really looking forward to making the build available for testing so as to get this fix into 2.4.2.1 bug-fix release.

[peanutbutterandcrackers]

Here it is!

[ferdnyc]

Here it is!

Hmm. Just a Windows installer? I was kind of hoping we'd get builds for all three platforms. I'd really like to get feedback on MacOS as well, if anyone's able to test there.

Also, it's built against 2.4.1? Is that right?

...In fact, I notice that there are (regular) daily builds produced after the 2.4.2 release versions that still indicate they're built from 2.4.1, which seems very strange.

...In addition, while I see the 2.4.2 release AppImage in there, it looks like the 2.4.2 Windows and Mac release builds don't appear on that list; they're only on https://www.openshot.org/download/ (as links to https://github.com/OpenShot/openshot-qt/releases/tag/v2.4.2)

Sorry, I realize 90% of this has absolutely nothing to do with this PR.

[ferdnyc]

Hmm. Just a Windows installer? I was kind of hoping we'd get builds for all three platforms. I'd really like to get feedback on MacOS as well, if anyone's able to test there.

(Also, at least 90% — possibly 100% — of the people reporting problems with the old tutorial code were running under Linux. I don't think we ever had a problem report from a Windows user.)

[peanutbutterandcrackers]

Sorry. Okay I'll get right on it. I just have a really really really bad Internet connection. It might take me a whole day (or perhaps two).

No. It is build from another branch that was forked from develop and then had this PR merged into it. This branch.

[ferdnyc]

Sorry. Okay I'll get right on it. I just have a really really really bad Internet connection. It might take me a whole day (or perhaps two).

You're creating them all locally? Yikes. That sounds no fun, especially with a less-than-fast connection. Well, good luck. I'd offer to help, but my impression is that the current build environment is not exactly portable, and I wouldn't expect that I could generate builds on my local systems that even remotely resemble the official ones. (Maybe I'm wrong?)

No. It is build from another branch that was forked from develop and then had this PR merged into it. This branch.

OK, but that branch is 2.4.2, at least according to its history. It contains all of the released commits. So I'm still not clear on why the file got named OpenShot-v2.4.1-whatever-x86_64.exe.

[peanutbutterandcrackers]

The following are the beta-builds that contain the changes made to resolve this issue (PR #1726 by the awesome @ferdnyc). Please try them out and report whether this resolves the issue. Thanks!

1. Windows Builds
2. Mac Builds
3. Linux Builds

[peanutbutterandcrackers]

Commenting that to copy-paste into the issues later on.

Not really. I have been given access to GitLab and I am downloading the artifacts from there and then re-uploading them to google drive and sharing the link here. :)

[ferdnyc]

Oh, good. I was worried the process was poor, when actually it's completely insane. 👍

[ferdnyc]

(Also, none of that explains why the files are named v2.4.1 when they're actually 2.4.2, nor why https://github.com/OpenShot/openshot-qt/releases/tag/daily contains (at the bottom) files newer than the 2.4.2 release that are still labeled 2.4.1. I think the build system is still stuck in the past, file-naming wise.)

[ferdnyc]

It seems to have something to do with the grab screen PR that was merged recently.

Not directly ­— that's probably my fault. In conflict-resolving the .ui file, I probably dropped @N3WWN's additions to it accidentally. I hate line-endings conversions. I'll fix it up now.

[ferdnyc]

@peanutbutterandcrackers The missing-action issue should be fixed now.

[ferdnyc]

I checked this out this morning to no luck. There is still black background with the tutorial pop-ups. On another hand, it is also there with the daily builds so it might be just some config on this machine.

Wait, really? The ones that don't have my code in them? But the issue isn't there with 2.4.2 release, or with 2.4.1????

It can't be just your machine, several other people have reported the same issue. WTF is going on with this?

[peanutbutterandcrackers]

This is what the tutorial window looks like for me in my daily PPA version - note the newly added screen capture feature; so 2.4.2+ (on Elementary 0.4.1 machine):

And this what the one from your branch looks like:

What's more... it seems that the tutorial pop-up is resizable (in your branch, of course) but I have to careful about my mouse placement. I see the drag-able tutorial pop ups have not yet been implemented...

[peanutbutterandcrackers]

Also, I see that you have resized the window (in a commit); but I can't really perceive any changes, for some reason... Could you please give me a few screenshots showing what is supposed to happen so that I can look for that change in my own machine too?

Thank you for taking the time with such an annoying feature. I really appreciate it. You're awesome.

[ferdnyc]

Also, I see that you have resized the window (in a commit); but I can't really perceive any changes, for some reason... Could you please give me a few screenshots showing what is supposed to happen so that I can look for that change in my own machine too?

No need, you've already done so yourself. The difference is between this:

and this:

...With the second screenshot, the Timeline is tall enough that the Timeline tutorial popup will appear in the correct place, instead of being pushed out of position by the bottom of the screen.

Now, because the window is apparently taller than your screen's y-resolution, that height is gained by sacrificing some of the height of the upper section, which means the Preview is smaller. That's unfortunate, but there's no other choice if we want the tutorial to properly illustrate the Timeline. On my screen, which is 1200px tall, OpenShot runs windowed with plenty of extra space below it, so the change just makes the window taller (and by extension, makes the Timeline taller) without shrinking the upper panels.

...I can't believe this latest change didn't fix the transparency bug, though. I really thought that was going to work.

What code exactly is in the PPA builds?

• Straight develop branch (old tutorial code)?
• This ferdnyc/tutorial2 branch (latest fixes)? As of when?
• Your test/ferd_tutorial2 branch in the upstream repo?
  • (That one is 6 days behind this branch, currently, and doesn't have those most recent fixes. But it does have my first attempted fix in 249d18f and 1af4094, so perhaps that actually made things WORSE instead of better?)

I need to know exactly what commits are included in any of these builds if I'm going to understand the results of any testing.

[ferdnyc]

Hmm. The only tutorial code change that develop does have is my 1b41150, attempting to fix the Tutorial window type in response to #1080 (the one that started me down this horrible road). It's possible THAT somehow broke transparency right from the beginning, for reasons I DO NOT understand (it's never been a problem on my system)... if so, and if the same issue is affecting the QDockWidget version of the tutorial, then I may have been on a losing path from the very beginning, and this was all wasted work because the Tutorials won't work (with transparency) as a dock or dialog window. That would... really suck.

But, we could revert 1b41150 and get transparency back... at the cost of the tutorial still not functioning properly for all of the people who've been having problems with it. (But the 1b41150 change didn't fix any of those, anyway. It looks like maybe the only thing it accomplished was to break transparency, for some, in the still-broken popups.)

If that's the case, I give up, and I'll delete this branch and move on.

[peanutbutterandcrackers]

No, I think this is an issue that is manifesting itself only on my machine.

The PPA does it from the develop branch. If indeed your hypothesis were true cue Sherlock Holmes music here, I would have experienced this issue long before. It has only been 2/3 days since I have seen the opaque background. I have been testing from your branch for quite a long time but it wasn't there then. I will try to test this out from other machines too, to see if I can reproduce this issue.

However, in light of this good news, I think your solution works wonderfully, and what you have done for OpenShot here is really really really great job (as always). So please do not even think about deleting this branch, good sir. cue in If we hold on together

Also, I just git checkout v2.4.1 and even then the black thingy issue persists for me.

I must have messed up something. 😄

I wonder if this is ready to be merged in now. I'll do a few more testing. I'll even get the newer builds, if you want me to, sir. 👍

[DylanC]

@ferdnyc @peanutbutterandcrackers - This tutorial fix works fine for me and seems to be confirmed as working here #1708. I would be happy to merge it. 👍

[peanutbutterandcrackers]

@DylanC - There are still plans - to make the tutorial windows drag-able, Cap'ain. I think we should wait for some more time. And also there's the issue of 'transitions' tutorial popping when view > views > simple view is clicked. Once those are fixed, perhaps we can merge this in. 👍

[DylanC]

@peanutbutterandcrackers - No problem. I feel maybe @ferdnyc regrets starting work on this! 😆

[DylanC]

@ferdnyc - Do you have much plans for the remainder of this PR?

[peanutbutterandcrackers]

@ferdnyc - I am also eagerly waiting for this PR to be merged (as soon as it is completely done) - after all the good work that you put into this. 😄

[ferdnyc]

@ferdnyc - Do you have much plans for the remainder of this PR?

Not at this particular moment, no.

@ferdnyc - I am also eagerly waiting for this PR to be merged (as soon as it is completely done) - after all the good work that you put into this.

So, I'll be honest. This is the first time I've even looked at this (the PR, my tutorial2 branch, any/all of it) since my previous Jul 11 comment, and I've been a happier person for it.

• I got the new translation-loading code wrapped up and I'm pleased with how that went.
• I finally had a chance to actually try out @N3WWN's ReSVG patches for libopenshot, which are awesome, and I'm hoping when he's available we can discuss the suggestions and proposals I left on that PR, I have some ideas for CMake integration there that I think will make life a lot easier for people building libopenshot locally, either as part of an OpenShot build or to for other uses.
• The past couple of days I've been working on cleaning up the logging (not only the messages, but the overall infrastructure itself) and that's going well: Just today I cracked something I'd had in my head for a few weeks now, asymmetrical logging.*

I haven't thought about this mess in days, and I'm still not ready to. I hate it, I've always hated it, and I've become convinced at this point that it's been going in a direction that was never going to work and most of it was time wasted.

Even if that's the case, there are some good things in here, like the changes to child-window management that allows OpenShot to be minimized without leaving floating docks lying around on the screen. If the PR won't end up being merged in this form (which I suspect will be its fate, as things stand) then at some point I'll need to go in and pull those changes out so I can submit them as their own self-contained PRs.

But I'm not ready yet. I hate this PR, I hate every line of this code, and I hate the fucking tutorials.

I'm gonna go delete more debug printfs now.

[ferdnyc]

I hate it, I've always hated it, and I've become convinced at this point that it's been going in a direction that was never going to work and most of it was time wasted.
If the PR won't end up being merged in this form (which I suspect will be its fate, as things stand)
I hate this PR, I hate every line of this code, and I hate the fucking tutorials.

And let me be clearer about what I mean by that: I don't think this PR should ever be finished and merged. I think the tutorials should be ripped out of the code completely.

Yeah, the old code worked fine for 90% of users, but even then 90% of those users never even looked at them. And for the other 10%, the bugs have interfered with them using OpenShot completely. The new code appears to have a similar track record on bugs, though fortunately the bugs are less likely to lock users out of the software completely. But it's still broken at about the same rate as the old code. I don't have any plans to fix it, because I don't know how to fix it, and I'm not sure it can be fixed. I'm damn sure it's a waste of time to try.

The amount of time we've all wasted dealing with tutorial bug reports, fighting to make these things work for the 10% they cause problems for... all for six short paragraphs that could easily be replaced with a Tutorial webpage containing a few screenshots. Which is exactly what I think should be done. Hell, they could even be screenshots of the tutorial popups, from a system where they work, before we rip them out.

Because I hate them, and I want them out of my life forever.

[peanutbutterandcrackers]

Work in limbo. Haha. 😄

It's completely all right. You take the time, good sir. You've already worked way too hard on this.

@DylanC - To be honest, I think this PR, even as it is right now, is better than what we currently have. I wouldn't really mind merging it in. But perhaps let's respect the good man's hard work and let him put his finishing touches - when he is ready - to this PR and then get this merged? Or we can do it in 2 separate installations too.

But I can confirm that even as it is, this is better than what we have now. A lot better, actually. No flickering and stuff as confirmed by several users, no lags, no unresponsiveness. Works well. (unlike what we currently have)

👍

@ferdnyc -

[ferdnyc]

I forgot about my asymmetrical-logging asterisk.

* - Asymmetrical logging:

Today I finally cracked configuring the logging level on the openshot_qt.log file separately from the console output, so that it's possible to feed MORE data into the log file than is output to the console. The way I have it set up right now, I've got a patch ready (but not pushed to #1772 yet) that does all of the following:

1. Sets the console output and the logfile up for INFO messages and higher, by default.
2. Connects up the debugging-mode preference so it affects OpenShot's logging as well, not just the proxy logging from libopenshot.
3. When debugging is enabled, it sets the logfile up to receive messages of DEBUG severity and higher, which means all of the debug spam gets poured into openshot_qt.log without also being spewed to the console.

This opens up a whole other world of possibilities, too. Like, instead of drowning openshot-qt.log with debug messages when debugging is enabled, we could leave that file receiving INFO-and-higher severity logs, and instead direct the DEBUG firehose to a separate openshot-debug.log file that's only created when debugging is turned on. That way, the normal logs for a session would still be available in relatively concise form even when debugging is enabled, and they could serve as a sort of "outline" to the var more verbose companion debug log.

Tons of possibilities, and I'd love to hear any ideas anyone has along those lines. Whereas any thoughts on the tutorials, I invite you to keep between you and your deity.

[DylanC]

@ferdnyc - I love the WIL (Work In Limbo) tag 😆

From what I've been reading from @peanutbutterandcrackers. He seems in favour of merging this PR. I feel from your own comments that you are looking to solve every single tutorial issue with this PR and maybe even looking for perfection.

I know you feel in its current state its still broken but I think its much less broken than what we currently have. Would you be able to solve the conflict with main-window.ui and we can get this in? (and move on with our lives)

It can always be revisited at a later stage.

[ferdnyc]

I feel from your own comments that you are looking to solve every single tutorial issue with this PR

I absolutely am, and I'll tell you why: The moment this code gets merged in, the tutorials become "my code". Even though I've still (as of this PR) written less than half of it. Every single tutorial bug that gets reported from that moment onward, I am going to get tagged on. And I cannot deal with that. I won't deal with that. Like I've said, my goal with this PR is to get the tutorials out of my life forever (because I fucking hate them), which halfway fixes won't accomplish. So, if I'm going to end up "owning" the tutorial code, it has to actually be good enough to support. This still isn't.

I don't believe in perfection, though. It doesn't exist.

I know you feel in its current state its still broken but I think its much less broken than what we currently have.

That... may be true, actually. See, I thought I'd introduced new bugs, and I was strongly of the opinion that the replacement code should at least not be worse than the old code in any way, or what's the point? But now it sounds like the transparency issues might be solved, at least for everyone who's not seeing the same transparency issues with the old code. (That, I cannot even remotely explain.)

And then there's this:

...Oh, I see! After you've completed the initial set of tutorials, but still have the ones for the advanced-layout panels pending, selecting "Simple View" causes the Transitions tutorial to pop up in the upper-left corner of the screen, even though Transitions still isn't visible in the interface! Is that what you meant? ...Regardless, I just encountered that myself. That's really strange, I'll have to look into it. Should be something fairly simple, probably just a state-check that's missing somewhere.

Again, I thought that was a bug that I'd introduced, but I just tested with 2.4.2 release and there again the same bug is present, meaning it's in the old part of the code that I didn't write. And yet, even *I* assumed that it was my fault. That is exactly the situation I'm trying to avoid, in becoming de facto owner of this trash code that I hate and also is terrible.

Would you be able to solve the conflict with main-window.ui and we can get this in?

That's weird, I already had solved those conflicts once. I wonder what brought them back?

It can always be revisited at a later stage.

The HELL I will.

[DylanC]

@ferdnyc

That... may be true, actually. See, I thought I'd introduced new bugs, and I was strongly of the opinion that the replacement code should at least not be worse than the old code in any way, or what's the point? But now it sounds like the transparency issues might be solved, at least for everyone who's not seeing the same transparency issues with the old code. (That, I cannot even remotely explain.)

I feel you think there is more wrong with your code than reality. I've seen most people just say it works and fixed it for their issue. It works for me too and I'm happy with it.

Again, I thought that was a bug that I'd introduced, but I just tested with 2.4.2 release and there again the same bug is present, meaning it's in the old part of the code that I didn't write. And yet, even I assumed that it was my fault. That is exactly the situation I'm trying to avoid, in becoming de facto owner of this trash code that I hate and also is terrible.

Yep, again not your code and not your fault. Some more paranoid thoughts creeping in. 😉

That's weird, I already had solved those conflicts once. I wonder what brought them back?

I'm not sure, I've been looking into it a little but it must have been a recent merge. The conflict resolution has the whole previous file in it, so its hard to judge what change is conflicting.

The HELL I will.

Tbh I don't think you will have to revisit this change. Its unlikely from what I can see so far.

[DylanC]

@ferdnyc - I think the line endings change has impacted this one.

[DylanC]

@ferdnyc - Fixed the merge conflict and pushed it in. Overall most people are happy with this change and I think that is a good enough reason to merge. I think you are the only person not 100% happy but trust the feedback from users. This PR is a good one. 👍