> On Jan. 22, 2015, 7:47 a.m., David Faure wrote: > > kdeui/itemviews/krecursivefilterproxymodel.cpp, line 149 > > <https://git.reviewboard.kde.org/r/120119/diff/1/?file=310592#file310592line149> > > > > refreshAll isn't used anymore, the new code always "refreshes all > > ascendants"
The default is false which is used mostly, except on line 257 - Christian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120119/#review74519 ----------------------------------------------------------- On Jan. 22, 2015, 3:11 p.m., Christian Mollekopf wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120119/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2015, 3:11 p.m.) > > > Review request for kdelibs and Stephen Kelly. > > > Bugs: 338950 > http://bugs.kde.org/show_bug.cgi?id=338950 > > > Repository: kdelibs > > > Description > ------- > > (This problem probably applies to KF5 as well, and we'll need to forward port > this patch.) > > > KRecursiveFilterProxyModel: Fixed the model > > The model was not working properly and didn't include all items under > some circumstances. > This patch fixes the following scenarios in particular: > > * The change in sourceDataChanged is required to fix the shortcut condition. > The idea is that if the parent is already part of the model (it must be if > acceptRow returns true), > we can directly invoke dataChanged on the parent, resulting in the changed > index > getting reevaluated. However, because the recursive filterAcceptsRow version > was used > the shortcut was also used when only the current index matches the filter and > the parent index is in fact not yet in the model. In this case we failed to > call > dataChanged on the right index and thus the complete branch was never added > to the model. > > * The change in refreshAscendantMapping is required to include indexes that > were > included by descendants. The intended way how this was supposed to work is > that we > traverse the tree upwards and find the last index that is not yet part of the > model. > We would then call dataChanged on that index causing it and its descendants > to get reevaluated. > However, acceptRow does not reflect wether an index is already in the model > or not. > Consider the following model: > > - A > - B > - C > - D > > > If C is include in the model by default but D not and A & B only gets > included due to C, we have the following model: > - A > - B > - C > - D > > If we then call refreshAscendantsMapping on D it will not consider B as > already being part of the model. > This results in the toplevel index A being considered lastAscendant, and a > call to dataChanged on A results in > a reevaluation of A only, which is already in the model. Thus D never gets > added to the model. > > Unfortunately there is no way to probe QSortFilterProxyModel for indexes that > are > already part of the model. Even the const mapFromSource internally creates a > mapping when called, > and thus instead of revealing indexes that are not yet part of the model, it > silently > creates a mapping (without issuing the relevant signals!). > > As the only possible workaround we have to issues dataChanged for all > ancestors > which is ignored for indexes that are not yet mapped, and results in a > rowsInserted > signal for the correct indexes. It also results in superfluous dataChanged > signals, > since we don't know when to stop, but at least we have a properly behaving > model > this way. > > > Diffs > ----- > > kdeui/itemviews/krecursivefilterproxymodel.cpp > 6d6563166bcc9637d826f577925c47d5ecbef2cd > kdeui/tests/CMakeLists.txt f661b9177a6e0e1de7f49bc3cb9fbb5e04f427c1 > kdeui/tests/krecursivefilterproxymodeltest.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/120119/diff/ > > > Testing > ------- > > > Thanks, > > Christian Mollekopf > >
