Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

saves the correct subwindow size when it is hidden #3589

Merged
merged 4 commits into from Jun 13, 2017

Conversation

@BaraMGB
Copy link
Contributor

@BaraMGB BaraMGB commented May 29, 2017

fixes #3584

@Wallacoloo
Copy link
Member

@Wallacoloo Wallacoloo commented May 30, 2017

Now that the sizeIfInvisible parameter is unused, it should probably be removed from the function signature as well.

@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented May 30, 2017

@qnebra
Copy link

@qnebra qnebra commented May 30, 2017

Possibly bug, how to reproduce:

  1. Open lmms with this branch merged
  2. New default project opened.
  3. Try open Pianoroll, Automation editor, Project notes,
  4. Nothing happens, they aren't shown or they just invisible. I don't know.
  5. Try open Song Editor, B+B Editor, Mixer, Controller Rack
  6. They opens like always will be.

GIF for proof:
http://imgur.com/fzemcyU

@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented Jun 1, 2017

@qnebra I can't reproduce. :( More people needed for testing here! @tresf @Umcaruje

@qnebra
Copy link

@qnebra qnebra commented Jun 1, 2017

Ok, maybe it's fixed and I just making unnecesary noise. Ok, you fixed this.

@Umcaruje Umcaruje added this to In Progress in Release 1.2.0 RC4 Jun 6, 2017
@@ -142,7 +142,7 @@ class MainWindow : public QMainWindow
return m_keyMods.m_alt;
}

static void saveWidgetState( QWidget * _w, QDomElement & _de, QSize const & sizeIfInvisible = QSize(0, 0) );
static void saveWidgetState(QWidget * _w, QDomElement & _de);

This comment has been minimized.

@zonkmachine

zonkmachine Jun 12, 2017
Member

Spaces at in the parenthesis.

Copy link
Member

@zonkmachine zonkmachine left a comment

I've tested this and I haven't noticed anything out of the ordinary. Fix the spaces (comment above) and merge this.

@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented Jun 12, 2017

@qnebra has objections. I talked to him in discord. He can reproduce the problems after checkout this branch. There seems to be something fishy.

@qnebra
Copy link

@qnebra qnebra commented Jun 12, 2017

I don't know exactly how it worked, in my case it look like "pianoroll", "automation editor", "project notes" windows dimensions are set to 0x0, so you can't find them, but they are active.

Maybe it's something specific, unable to reproduce.

@zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jun 12, 2017

@BaraMGB Can you rebase this against stable-1.2 ?
I added a default template in ac9f4a2 and if there is a bug with new projects/templates, this is going to affect this PR.

@qnebra
Can you paste back the output of git log --oneline -n 10 ?
Do you have a local default project template? (Save as default tempate?).
Can you try a clean build?

@qnebra
Copy link

@qnebra qnebra commented Jun 12, 2017

@zonkmachine

  1. git log --oneline -n 10
    805ed24 remove invisible size from saveWidgetState()
    a82462f saves the correct subwindow size when it is hidden
    f1a3368 Merge pull request #3583 from tresf/stable-1.2
    745042d Fix CMP0050 compilation warnings
    3607c92 Bitcrush Plugin Redesign (#3575)
    38bca9a fix time signature updates timelinewidget also on pos 0 (#3572)
    f8a508f Fixes #3563 (#3567)
    6970c88 fix alternate shading for changing time signature (#3559)
    c0b910e Replacement metronome samples (#3513)
    300058d Flanger LFO rate syncronised (#3521)

  2. Yes, I have.

  3. I will try.

Ok guys, very, very sorry for noise. Create a new one default template fixed issue locally.

@zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jun 12, 2017

Ok guys, very, very sorry for noise. Create a new one default template fixed issue locally.

No, that is a concern. I still think we need to test this with the new template fix.

@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented Jun 13, 2017

@zonkmachine done

@zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jun 13, 2017

Works fine here like before.

@qnebra Can you build and test this? Test it both with and without the template you made.

@qnebra
Copy link

@qnebra qnebra commented Jun 13, 2017

@zonkmachine Ok, I hope delete new default template will help test. Now install.
It's look like fixed, I haven't issue now.

@zonkmachine zonkmachine merged commit fc6d844 into LMMS:stable-1.2 Jun 13, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jun 13, 2017

That'll do. Merge! 🚜

@zonkmachine zonkmachine moved this from In Progress to Done in Release 1.2.0 RC4 Jun 13, 2017
@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented Jun 13, 2017

👍 Thanks to @qnebra and @zonkmachine !

tresf added a commit to liushuyu/lmms that referenced this pull request Jun 13, 2017
* saves the correct subwindow size when it is hidden

* remove invisible size from saveWidgetState()
@tresf
Copy link
Member

@tresf tresf commented Jun 13, 2017

Cherry-picked and ready for master in 220559f.

PhysSong added a commit to PhysSong/lmms that referenced this pull request Jul 8, 2017
* saves the correct subwindow size when it is hidden

* remove invisible size from saveWidgetState()
PhysSong added a commit to PhysSong/lmms that referenced this pull request Jul 8, 2017
* saves the correct subwindow size when it is hidden

* remove invisible size from saveWidgetState()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.