Skip to content

[WIP] Library Layout Redesign Single pane#1115

Closed
jmigual wants to merge 594 commits intomixxxdj:masterfrom
jmigual:feature/newSingleLibrary
Closed

[WIP] Library Layout Redesign Single pane#1115
jmigual wants to merge 594 commits intomixxxdj:masterfrom
jmigual:feature/newSingleLibrary

Conversation

@jmigual
Copy link
Copy Markdown
Contributor

@jmigual jmigual commented Jan 10, 2017

Hi this is a continuation of #991 with the changes made

  • Single pane visualization
  • Fix bug with playlists
  • Fix merge conflicts @jmigual
  • polish Auto DJ pane
  • Auto DJ crate icon is not colored
  • Deere's preview player is not recognizable as "player" if it is empty
  • No filter (Search) in browse feature

jmigual added 30 commits August 8, 2016 22:59
Also improved the query when selecting a header item
Conflicts:
	build/depends.py
	src/library/coverart.h
@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 10, 2017

Great work, thank you!!

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 11, 2017

This crashes as soon as I try to select a track in the Library table. gdb backtrace:

Warning [Main]: DEBUG ASSERT: "row < m_children.size()" in file src/library/treeitem.cpp, line 77
Fatal [Main]: ASSERT failure in QList<T>::operator[]: "index out of range", file /usr/include/QtCore/qlist.h, line 473

Thread 1 "mixxx" received signal SIGABRT, Aborted.
0x00007fffefc78765 in raise () from /usr/lib64/libc.so.6
(gdb) bt
#0  0x00007fffefc78765 in raise () at /usr/lib64/libc.so.6
#1  0x00007fffefc7a36a in abort () at /usr/lib64/libc.so.6
#2  0x00007ffff475082d in qt_message_output(QtMsgType, char const*) (msgType=msgType@entry=QtFatalMsg, buf=<optimized out>)
    at global/qglobal.cpp:2423
#3  0x00007ffff47509b1 in qt_message(enum QtMsgType, const char *, typedef __va_list_tag __va_list_tag *) (msgType=msgType@entry=QtFatalMsg, msg=msg@entry=0x7ffff48d29e0 "ASSERT failure in %s: \"%s\", file %s, line %d", ap=ap@entry=0x7fffffffca40)
    at global/qglobal.cpp:2469
#4  0x00007ffff47512d1 in qFatal(char const*, ...) (msg=msg@entry=0x7ffff48d29e0 "ASSERT failure in %s: \"%s\", file %s, line %d")
    at global/qglobal.cpp:2652
#5  0x00007ffff475134a in qt_assert_x(char const*, char const*, char const*, int) (where=where@entry=0xf3a88a "QList<T>::operator[]", what=what@entry=0xf33620 "index out of range", file=file@entry=0xf3397d "/usr/include/QtCore/qlist.h", line=line@entry=473)
    at global/qglobal.cpp:2126
#6  0x0000000000a4e2f6 in TreeItem::child(int) const (i=<optimized out>, this=<optimized out>) at /usr/include/QtCore/qlist.h:473
#7  0x0000000000a4e2f6 in TreeItem::child(int) const (this=this@entry=0x3813140, row=row@entry=3) at src/library/treeitem.cpp:78
#8  0x000000000095d86d in BasePlaylistFeature::slotTrackSelected(TrackPointer) (this=this@entry=0x3850010, pTrack=...)
    at src/library/features/baseplaylist/baseplaylistfeature.cpp:806
#9  0x000000000096492e in BasePlaylistFeature::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x3850010, _c=<optimized out>, _id=24, _a=<optimized out>) at lin64_build/library/features/baseplaylist/moc_baseplaylistfeature.cc:117
#10 0x00007ffff487f090 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) (sender=sender@entry=0x330a290, m=m@entry=0xf87880 <Library::staticMetaObject>, local_signal_index=local_signal_index@entry=4, argv=argv@entry=0x7fffffffcd10)
    at kernel/qobject.cpp:3567
#11 0x00000000009fcbf1 in Library::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_t1=<error reading variable: access outside bounds of object referenced via synthetic pointer>, this=0x330a290) at lin64_build/library/moc_library.cc:201
#12 0x00000000009fcbf1 in Library::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x330a290, _c=<optimized out>, _id=<optimized out>, _a=<optimized out>) at lin64_build/library/moc_library.cc:108
#13 0x00007ffff487f090 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) (sender=sender@entry=0x3472560, m=m@entry=0xf888c0 <LibraryFeature::staticMetaObject>, local_signal_index=local_signal_index@entry=8, argv=argv@entry=0x7fffffffceb0)
    at kernel/qobject.cpp:3567
#14 0x00000000009fe46a in LibraryFeature::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_t1=<error reading variable: access outside bounds of object referenced via synthetic pointer>, this=0x3472560) at lin64_build/library/moc_libraryfeature.cc:218
#15 0x00000000009fe46a in LibraryFeature::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x3472560, _c=<optimized out>, _id=8, _a=<optimized out>) at lin64_build/library/moc_libraryfeature.cc:104
#16 0x00007ffff487f090 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) (sender=0x42868ab0, m=m@entry=0xfc7960 <WLibraryTableView::staticMetaObject>, local_signal_index=local_signal_index@entry=3, argv=argv@entry=0x7fffffffcfe0)
    at kernel/qobject.cpp:3567
#17 0x0000000000c93a73 in WLibraryTableView::trackSelected(TrackPointer) (this=<optimized out>, _t1=...)
    at lin64_build/widget/moc_wlibrarytableview.cc:141
#18 0x0000000000d11c32 in WTrackTableView::slotGuiTick50ms(double) (this=this@entry=0x42868ab0) at src/widget/wtracktableview.cpp:179
#19 0x0000000000c9ba59 in WTrackTableView::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x42868ab0, _c=<optimized out>, _id=31, _a=0x7fffffffd190) at lin64_build/widget/moc_wtracktableview.cc:140
#20 0x00007ffff487f090 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) (sender=sender@entry=0x41586fa0, m=m@entry=0xf37100 <ControlProxy::staticMetaObject>, local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fffffffd190)
    at kernel/qobject.cpp:3567
#21 0x000000000052141f in ControlProxy::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_t1=<optimized out>, this=<optimized out>) at lin64_build/control/moc_controlproxy.cc:118
#22 0x000000000052141f in ControlProxy::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (pSetter=<optimized out>, v=<optimized out>, this=<optimized out>) at lin64_build/control/../../src/control/controlproxy.h:109
#23 0x000000000052141f in ControlProxy::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x41586fa0, _c=<optimized out>, _id=<optimized out>, _a=<optimized out>) at lin64_build/control/moc_controlproxy.cc:67
#24 0x00007ffff4884db1 in QObject::event(QEvent*) (this=0x41586fa0, e=<optimized out>) at kernel/qobject.cpp:1222
#25 0x00007ffff5289edc in QApplicationPrivate::notify_helper(QObject*, QEvent*) (this=this@entry=0x14086f0, receiver=receiver@entry=0x41586fa0, e=e@entry=0x7ffef4003a90) at kernel/qapplication.cpp:4565
#26 0x00007ffff5290ddc in QApplication::notify(QObject*, QEvent*) (this=this@entry=0x7fffffffd910, receiver=receiver@entry=0x41586fa0, e=e@entry=0x7ffef4003a90) at kernel/qapplication.cpp:4351
#27 0x0000000000a76772 in MixxxApplication::notify(QObject*, QEvent*) (this=0x7fffffffd910, target=0x41586fa0, event=0x7ffef4003a90)
    at src/mixxxapplication.cpp:137
#28 0x00007ffff486aeed in QCoreApplication::notifyInternal(QObject*, QEvent*) (this=0x7fffffffd910, receiver=receiver@entry=0x41586fa0, event=event@entry=0x7ffef4003a90) at kernel/qcoreapplication.cpp:955
#29 0x00007ffff486e556 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (event=0x7ffef4003a90, receiver=0x41586fa0) at kernel/qcoreapplication.h:231
#30 0x00007ffff486e556 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (receiver=receiver@entry=0x0, event_type=event_type@entry=0, data=0x13ef840) at kernel/qcoreapplication.cpp:1579
#31 0x00007ffff486e853 in QCoreApplication::sendPostedEvents(QObject*, int) (receiver=receiver@entry=0x0, event_type=event_type@entry=0) at kernel/qcoreapplication.cpp:1472
#32 0x00007ffff489b2ee in postEventSourceDispatch(GSource*, GSourceFunc, gpointer) () at kernel/qcoreapplication.h:236
#33 0x00007ffff489b2ee in postEventSourceDispatch(GSource*, GSourceFunc, gpointer) (s=0x140a670)
    at kernel/qeventdispatcher_glib.cpp:300
#34 0x00007ffff0b216ba in g_main_context_dispatch (context=0x1409910) at gmain.c:3154
#35 0x00007ffff0b216ba in g_main_context_dispatch (context=context@entry=0x1409910) at gmain.c:3769
#36 0x00007ffff0b21a70 in g_main_context_iterate (context=context@entry=0x1409910, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3840
#37 0x00007ffff0b21b1c in g_main_context_iteration (context=0x1409910, may_block=may_block@entry=1) at gmain.c:3901
#38 0x00007ffff489b45e in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (this=0x14097d0, flags=...)
    at kernel/qeventdispatcher_glib.cpp:450
#39 0x00007ffff53325b6 in QGuiEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (this=<optimized out>, flags=...) at kernel/qguieventdispatcher_glib.cpp:207
#40 0x00007ffff48697bf in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (this=this@entry=0x7fffffffd840, flags=...) at kernel/qeventloop.cpp:149
#41 0x00007ffff4869b25 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (this=this@entry=0x7fffffffd840, flags=...)
    at kernel/qeventloop.cpp:204
#42 0x00007ffff486f779 in QCoreApplication::exec() () at kernel/qcoreapplication.cpp:1227
#43 0x00007ffff528872c in QApplication::exec() () at kernel/qapplication.cpp:3823
#44 0x000000000049d0be in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:116

m_numSamplers.connectValueChanged(SLOT(slotNumSamplersChanged(double)));
m_numPreviewDecks.connectValueChanged(SLOT(slotNumPreviewDecksChanged(double)));

// Controls to navigate vertically within currently focussed widget (up/down buttons)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmigual
It looks like your merge strategy was just "delete everything"...? All of my changes to this file have been deleted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I look again and check what happened but your changes didn't give me any conflicts

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 11, 2017

I only looked at how the merge conflicts were handled on my latest commit to librarycontrol.cpp, but looking at that I don't feel that the git history of this branch is salvageable.

@daschuer: unless you're willing to re-fork the branch from #991 and carefully handle the merge conflicts one by one, I vote that we just pick the files / code we need from this branch and copy them into master one by one...

@jmigual
Copy link
Copy Markdown
Contributor Author

jmigual commented Jan 11, 2017

Your commit didn't give me any conflicts so I don't understand why your changes are deleted.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 11, 2017

Can you try this:

git checkout -b newSingleLibrary2 7dfd25f
git checkout master
git pull upstream master
git checkout newSingleLibrary2
git merge master
# handle merge conflicts
git cherry-pick eacfdea4c1f023b0e6d470a44ec335e0f2db70ef
git cherry-pick 5b145767e9a101d4397e18270c9ee5a4d6c11483
git cherry-pick 9e47eb66fa8e9ba33f4d605bca058256ff24022e
git cherry-pick 7dfd25f3d7d0367dcf95bc3ee3267ddccb5df3c1
git cherry-pick d11bf1e2be1b233f8fc1a437f2cad7868ac58658
git cherry-pick 90496437338f9df388fe5c7bdb3ae07947d2a4bf
git push origin newSingleLibrary2

@daschuer
Copy link
Copy Markdown
Member

I use SmartGit for merging. It has a nice 3 pane view. Maybe it helps out of this issue here as well.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 11, 2017

Yeah I've been going through tonight with meld which is the same I guess... There are heavy changes on both sides to the treeitem and treeitemmodel however, and it's not clear how to handle them.

@jmigual
Copy link
Copy Markdown
Contributor Author

jmigual commented Jan 11, 2017

Yes I used Visual Studio that does the same. In TreeItem I've decided to keep most of their changes since they were a nice addition.

@timrae I'll try this solution this Friday tonight.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 11, 2017

@jmigual
I've started the merge conflict resolution... Please see my branch. I've at least fixed all of the conflicts with my code haha.

I'm currently stuck with the TreeItem... Current master branch has dataPath() function so we need to implement that:

src/library/features/browse/foldertreemodel.cpp: In member function ‘virtual bool FolderTreeModel::hasChildren(const QModelIndex&) const’:
src/library/features/browse/foldertreemodel.cpp:48:14: error: ‘class TreeItem’ has no member named ‘dataPath’
     if(item->dataPath().toString() == QUICK_LINK_NODE)

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 11, 2017

@daschuer @jmigual
If you guys could clone my branch and help with fixing the compile errors that would be appreciated... We can work together on this without doing everything ourselves.

@jmigual
Copy link
Copy Markdown
Contributor Author

jmigual commented Jan 11, 2017

I'm currently stuck with the TreeItem... Current master branch has dataPath() function so we need to implement that:

No, actually the current master has no more dataPath() and that's why there are merge conflicts. The current tree item has getData() instead and the previous data() now it's getLabel().

This new TreeItem interface was added in #1067 . Actually for the TreeItem and TreeItemModel you can directly take the ones from this branch since I merged and fixed it myself because git was not able to do it.

@daschuer
Copy link
Copy Markdown
Member

OK, I will give it a try now. It is probably a good idea to merge PR by PR and crawl up the master branch. This way we have small review-able merge commits.

@jmigual
Copy link
Copy Markdown
Contributor Author

jmigual commented Jan 11, 2017

I'm already done fixing the compilation issues in @timrae's branch if you wait 30 mins I'll be finished

@daschuer
Copy link
Copy Markdown
Member

OK, than I will be patient ;-) Thank you very much!

@jmigual
Copy link
Copy Markdown
Contributor Author

jmigual commented Jan 11, 2017

I've added the changes in timrae#1 PR.

@daschuer
Copy link
Copy Markdown
Member

Grate, the latest changes are looking good. Thank you both. I will now start trying to move your both code into some clean merge commits, without changing the end result.
That should be a good base for our future changes.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

Thanks @daschuer
So you'll clean up, and open a new PR? Then going forwards we work on your branch?

(edit: cherry-pick was not required as @jmigual already took care of it 👍 )

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

I pushed another commit to my branch fixing the linking errors... Now mixxx starts up, though it crashes if you try to load a track from the library

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

Warning [Main]: DEBUG ASSERT: "row < m_children.size()" in file src/library/treeitem.cpp, line 77 Fatal [Main]: ASSERT failure in QList<T>::operator[]: "index out of range", file /usr/include/qt4/QtCore/qlist.h, line 473

@daschuer
Copy link
Copy Markdown
Member

The clean branch is in my personal repro. The rebase onto it with the latest commits is still missing.

@daschuer
Copy link
Copy Markdown
Member

Ok, the cleaned branch is ready: https://github.com/daschuer/mixxx/tree/jmigual-library-redesign
If it looks good to you, it would be nice, to reset your branch from this PR to it.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

I don't think 811cda9 should be there...? I don't feel it's ready, and as it was discussed, not having it doesn't add any regressions, right?

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

Also, I think it's better if you make a new PR as @jmigual said he's busy with exams, and he won't want to be checking our merge requests into his fork

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 12, 2017

@jmigual one more bug I noticed, is that in the playlists feature now, it shows the IDs instead of the name when you select that feature.

@daschuer
Copy link
Copy Markdown
Member

OK, I will do a new PR.

@daschuer daschuer mentioned this pull request Jan 12, 2017
11 tasks
@daschuer
Copy link
Copy Markdown
Member

Lets move to #1117

@daschuer daschuer closed this Jan 12, 2017
@jmigual jmigual deleted the feature/newSingleLibrary branch January 24, 2017 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants