Introduce QtCompat::QEnterEvent so enterEvent() overrides build on Qt5 & Qt6

[acolwell]

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

• I've followed the contributing guidelines to the best of my understanding
• I've read and understood the contributing guidelines
• I've formatted my code according to Natron's code style
• I've searched the pull requests tracker to ensure that this PR is not a duplicate

PR Description

What type of PR is this? (Check one of the boxes below)

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change which does not add functionality nor fixes a bug but improves Natron in some way)
• Breaking change (fix or feature that would cause existing functionality to change)
• My change requires a change to the documentation
  • I have updated the documentation accordingly

What does this pull request do?

The signature of QWidget::enterEvent() changed between Qt5 & Qt6. This change simply creates a typedef that provides the correct type based on which version of Qt we are building with.

Have you tested your changes (if applicable)? If so, how?

Yes. The Qt5 installer still builds. It is also consistent with and a little cleaner than the similar Qt6 changes in #1019 .

[YakoYakoYokuYoku]

It was agreed that RB-2.6 is Qt5 only, and in my opinion RB-2.7 should be Qt6 only. And I think that this alias could go in Gui/GuiFwd.h or a different file and not in a Global header as this is only for GUI.

[acolwell]

It was agreed that RB-2.6 is Qt5 only, and in my opinion RB-2.7 should be Qt6 only. And I think that this alias could go in Gui/GuiFwd.h or a different file and not in a Global header as this is only for GUI.

I don't think it is wise to make RB-2.6 Qt5 only and RB-2.7 Qt6 only especially in cases like this where it is relatively trivial to accommodate both with relatively simple changes. Making a hard rule just creates a bunch of merging work if we have fixes that should apply to both branches. It seems like we should be pushing off unnecessary branch work as long as possible. Given the level of activity on this repo it seems like maintaining a single branch for as long as possible is more likely to succeed and will reduce confusion for developers who might want to contribute.

I can understand your perspective about perhaps putting this in GuiFwd.h, but I decided to put in Global/QtCompat.h because that was already established and I thought it might make sense to have one header for Qt related compatibility issues. Yes this change only applied to Gui code , but a likely change related to QMutexLocker would likely be needed in both Engine and Gui code. It seemed to me like it would be good to keep these Qt related changes together independent of which specific submodule uses them. Qt is used across submodules so it seems like compatibility functionality should be in a cross-module location like Global. This seems to be the intent of the original Global/QtCompat.h and so I was just trying to follow that lead.

[YakoYakoYokuYoku]

Still, RB-2.6 has to be Qt5 only. I acknowledge that #1020 was merged into said branch but that was before it was decided to only support that version. And yes, it isn't unsensical to make releases a sole version of Qt, even QGIS did this for their 4.0 release. While I've said that RB-2.7 should be Qt6 only it was my opinion nonetheless and if merging is needed then I should be the one doing that because I was the one committing to it, you can take my word for it. Though, I cannot accept this PR due to the scope of the RB-2.6 branch.

[acolwell]

This is an odd flex. The Qt5-only requirement seems arbitrary and not likely to improve development. I'm just trying to make progress on something that has been dormant for 4 months. Also given that I have made the majority of the changes on the 2.6 branch I assure you I'm fine with some Qt6 logic like this going on that branch. The changes I've made here do not cause major issues with readability or maintainability and doing so helps us avoid unnecessary merging.

[YakoYakoYokuYoku]

This is an odd flex. The Qt5-only requirement seems arbitrary and not likely to improve development.

That was said in this comment, I can admit an innacurate interpretation from my part but I've changed the base branch there to accommodate what I was told to.

I'm just trying to make progress on something that has been dormant for 4 months.

But I don't agree with this PR, using Global for a Gui only one time change isn't appropriate. Everything shall be put in Gui and if a namespace is needed, which I don't see too much of a point for a type alias but regardless, then using namespace has to be there too.

Also given that I have made the majority of the changes on the 2.6 branch I assure you I'm fine with some Qt6 logic like this going on that branch. The changes I've made here do not cause major issues with readability or maintainability and doing so helps us avoid unnecessary merging.

Though you've said that you had encountered issues with the version change and I don't want to see them on this branch. Unless it's decided that there's no problem of dealing with Qt6 changes then those changes can go, otherwise just use the RB-2.7 branch.

[acolwell]

This is an odd flex. The Qt5-only requirement seems arbitrary and not likely to improve development.

That was said in this comment, I can admit an innacurate interpretation from my part but I've changed the base branch there to accommodate what I was told to.

So this seems selective since a following comment, which outlines the position I'm taking here, indicates the preference for changes like this to go on 2.6 and it got a thumbs up by both you and @devernay . I took that as agreement.

I'm just trying to make progress on something that has been dormant for 4 months.

But I don't agree with this PR, using Global for a Gui only one time change isn't appropriate. Everything shall be put in Gui and if a namespace is needed, which I don't see too much of a point for a type alias but regardless, then using namespace has to be there too.

You haven't really responded to my response about why I picked Global. You've just restated your preference from Gui seemingly without taking into account the new information.

While technically the namespace isn't needed I've put it there to explicitly call out that this is not the usual QEnterEvent type. Eventually when we get to a "Qt6 and beyond only" position we can just simply remove the QtCompat:: from all the methods with something like sed and remove the typedef. I'm going for the goal of "least surprise" and as such I'm being more explicit about the typename then is strictly necessary.

Also given that I have made the majority of the changes on the 2.6 branch I assure you I'm fine with some Qt6 logic like this going on that branch. The changes I've made here do not cause major issues with readability or maintainability and doing so helps us avoid unnecessary merging.

Though you've said that you had encountered issues with the version change and I don't want to see them on this branch. Unless it's decided that there's no problem of dealing with Qt6 changes then those changes can go, otherwise just use the RB-2.7 branch.

Yes. I have seen issues. There has also been changes to the typesystem code since I did that experiment (specifically #1020 ) and so I don't know if those concerns are still valid. That is part of the reason why I've been advocating for landing some of these simpler and non-invasive changes, in smaller PRs, on 2.6 so that we can cut down the differences and make exploring this easier. If we find that something super disruptive is needed, then fine we can just land it on 2.7. From what I've seen though most of the changes are either simple typedef issues or just refactoring to use more modern classes like QRegularExpression which work in both Qt5 and Qt6.

I don't really understand why this is contentious. I'm just trying to facilitate progress here in a way that I believe will avoid unnecessary extra work.

[devernay]

• Rb-2.6 must build ok with Qt5, and it's nice if it also build with Qt6
• All commits to RB-2.6 should be merged into RB-2.7, as we expect bugs to be fixed in RB-2.6, which is yet to be released
• Qt6-specific stuff can go into RB-2.7

I think it's easier to make RB-2.6 as much as possible Qt6-compatible. We can always clean up Qt5-specific stuff from RB-2.7 later (when 2.6 is released for example)

[acolwell]

Thanks for the review..