Qt5/KDE5 bad interactions

classic Classic list List threaded Threaded
49 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

D. Michael McIntyre-3
On 02/27/2017 07:19 AM, David Faure wrote:

> No need to bisect, it's clear that this is related to my redesign of the
> parameter area (was a (useless) stack widget inside a dockwidget,
> now it's a simple widget in a layout, the only thing I kept was that it's a
> scrollarea). I didn't think of testing with a document as cmdline param.

I've been all through what you did there, and I don't see what you missed.

--
D. Michael McIntyre

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

David Faure
In reply to this post by David Faure
On lundi 27 février 2017 13:19:16 CET David Faure wrote:

> On lundi 27 février 2017 13:09:11 CET Ted Felix wrote:
> > On 02/26/2017 11:28 PM, D. Michael McIntyre wrote:
> > > Load a document, and dozens of things break instantly.  This is
> > > infinitely repeatable here.  (Only testing with Qt5 builds here.)
> > >
> >    Ah ha!  That's it.  I'm seeing this with Qt4 too.  Should be easy to
> >
> > bisect and find.  I'll take a look sometime this week.
>
> No need to bisect, it's clear that this is related to my redesign of the
> parameter area (was a (useless) stack widget inside a dockwidget,
> now it's a simple widget in a layout, the only thing I kept was that it's a
> scrollarea). I didn't think of testing with a document as cmdline param.

Found it. Nasty nasty use of QObject::blockSignals(true).
This blocking was done on "all children of the old view" before swapping to
the new view, which now includes the parameter area that will get reparented
into the new view (before my change the parameter area was a dockwidget, so
not a child of the view). Obviously after that blockSignals(true) nothing
works in the parameter area, it has all its signals blocked forever.

Does anyone remember the actual reason for this code?
The comment is not specific enough.

void
RosegardenMainWindow::initView()
{
    [...]
    // The plan is to set the new central view via setCentralWidget() in
    // a moment, which schedules the old one for deletion later via
    // QObject::deleteLater().  However, we need to make sure that the old one
    // behaves as if it's already been deleted -- i.e. that it and its
    // entire tree of children send no signals between now and its
    // actual deletion.
    //
    RosegardenMainViewWidget *oldView = m_view;
    if (oldView) {
        oldView->blockSignals(true);
        // a qt-ism that I have never actually used before.  brief innit
        foreach (QObject *o, oldView->findChildren<QObject *>()) {
            o->blockSignals(true);
        }
    }

Which signals exactly is this trying to prevent against? Not much code will
happen between here and the view swapping anyway.

I vote for killing all this blockSignals(true) usage, but the question is what
happens then. In my quick test (start with cmdline arg, then open another
doc), it all seems to work fine without the above code, but presumably it was
added for a reason?

--
David Faure, [hidden email], http://www.davidfaure.fr
Working on KDE Frameworks 5


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

David Faure
On mercredi 1 mars 2017 01:08:55 CET David Faure wrote:
> presumably it was added for a reason?

OK, git log being much more convenient than svn to figure this out I had a
quick look, and here's the commit that introduced it:

r11067
Author: cannam
Date:   Tue Oct 20 11:38:30 2009 +0000

    * Fix a further crash on document load.  This is probably the crash that
      has been troubling Emanuel.  The cause was a signal from a parameter box
      to the old view -- although the parameter boxes were connected up to the
      new view on load, they were still also connected to the old one.  Fix:
      disconnect them.  We're really piling up the fixes here for this gross
      little bit of object structure.

If anyone knows how to reproduce this crash, I'm interested.

>From the description it seems to me that the other part of the commit,
 + disconnect(m_segmentParameterBox, 0, oldView, 0);
 + disconnect(m_instrumentParameterBox, 0, oldView, 0);
 + disconnect(m_trackParameterBox, 0, oldView, 0);
... was enough, no need for blockSignals on the view and its children.

--
David Faure, [hidden email], http://www.davidfaure.fr
Working on KDE Frameworks 5


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

Ted Felix-2
In reply to this post by David Faure
On 02/28/2017 07:08 PM, David Faure wrote:
> I vote for killing all this blockSignals(true) usage,

   I second this.  I've been removing every blockSignals() that I come
across.  So, if you can get rid of it, please do.

> but the question is what
> happens then. In my quick test (start with cmdline arg, then open another
> doc), it all seems to work fine without the above code, but presumably it was
> added for a reason?

   That was so long ago....  We've restructured a lot of stuff in the
meantime and it's entirely possible that it's safe to do away with it.
I vaguely recall simplifying some of the code related to doc/view
swapping, so I might have shuffled things the right way.

   I'll take a peek at the mailing lists around this time and see if I
can find out what the test case was for the commit that introduced this.

Ted.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

Ted Felix-2
In reply to this post by David Faure
> r11067
> Author: cannam
> Date:   Tue Oct 20 11:38:30 2009 +0000
>
>     * Fix a further crash on document load.  This is probably the crash that
>       has been troubling Emanuel.

   The closest thing I can find in the mailing list was on Oct 8 2009.
Emanuel reported a crash when opening an rg 1.7.3 file.  Though that
appears to be a problem in TrackButtons, not anything in the parameters
boxes.  "has been troubling" leads me to believe that it might have been
prior to this, though I found no posts from Emanuel in September 2009.

   My guess is that opening a regular rg file and/or importing a MIDI
file should be enough to shake this out.  Those are the most common
cases anyway.

Ted.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

D. Michael McIntyre-3
In reply to this post by Ted Felix-2
On 02/28/2017 07:41 PM, Ted Felix wrote:

>    That was so long ago....  We've restructured a lot of stuff in the
> meantime and it's entirely possible that it's safe to do away with it.

It is entirely possible.  You've made a lot of progress in that area.

>    I'll take a peek at the mailing lists around this time and see if I
> can find out what the test case was for the commit that introduced this.

I never did find the discussion or the test case, but that name came up
again and again.  It was a collection of bad memories reminding me why I
should never, ever take a management job in the real world.  Ugh.

I'm comfortable with ignoring the past on this one and worrying about
dealing with any present and future consequences.

--
D. Michael McIntyre

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

David Faure
On mercredi 1 mars 2017 06:31:15 CET D. Michael McIntyre wrote:
> I'm comfortable with ignoring the past on this one and worrying about
> dealing with any present and future consequences.

Done.

--hacks;

--
David Faure, [hidden email], http://www.davidfaure.fr
Working on KDE Frameworks 5


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

D. Michael McIntyre-3
On 03/01/2017 03:25 AM, David Faure wrote:

> Done.
>
> --hacks;

I tested several different scenarios, and this problem is throughly solved.

I should get time tomorrow to finish that bit of file dialog hackery,
and then I think this is stable enough to release.

--
D. Michael McIntyre

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
Reply | Threaded
Open this post in threaded view
|

Re: Qt5/KDE5 bad interactions

Ted Felix-2
In reply to this post by David Faure
On 02/25/2017 12:08 PM, David Faure wrote:
>>> Also, the Manage MIDI Banks and Programs window is larger.  On my 768
>>> vertical display, I can't see the Options section in the lower right.
>>> In 16.06 the entire dialog fits on my screen.
> This dialog should be OK now.

   It's better, but still there's something odd.  When I first launch
it, it fits fine in 768 vertical.  Then if I move the window, it expands
ever so slightly vertically such that the buttons at the bottom are off
the screen.  I can work around this by maximizing the window.

   I can't see anything specific that changes when I move the window.
The various pieces just become slightly larger.  It's like the layout is
expanding to fill a slightly larger window.  (Expansion also occurs if I
select a bank.)

   Can we limit this window's height to a maximum of 744 pixels
(including its titlebar, 716 without)?  That's the available space at
768 vertical with the Unity top bar.

Ted.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
_______________________________________________
Rosegarden-devel mailing list
[hidden email] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
123