> On Aug. 15, 2013, 5:16 p.m., Albert Astals Cid wrote: > > Thanks for the patch! > > > > The enum thing seems a bit overkill to me, i mean, how "expensive" is the > > updateSample function after all? Personally, I'd just call it and that's > > it. Also makes sure you don't introduce subtle bugs in the future if > > someone expands a function to do something else but forgets to add an | enum > > > > Anyone else has an opinion? > > Santeri Piippo wrote: > Without the filtering I had some severish delay when inputting, like half > a second with every key press since it had to update and compute so much > stuff. The enum does help things. > > > Also makes sure you don't introduce subtle bugs in the future if > someone expands a function to do something else but forgets to add an | enum > This already kind of exists from how the updates already need to be > called from various slots.. if someone figures that they should update the > sample, figuring out that an enumerator needs to be added gets kind of > obvious. > > In retrospect though it does seem a bit overkill but I don't know of a > better way to filter things like that... 4 bools would just be flat out > confusing IMO. > > Albert Astals Cid wrote: > Really? half a second? That seems like a lot, what is taking so much? > > Santeri Piippo wrote: > Well to be more precise, the UI stutters when you input data, so I > figured to do some performance measures. It's not half a second but it's > definitely noticable.
actually nevermind this, I'm just slow here; it's not the sample updating which causes the stuttering, there's a lot of other stuff that updating certain things in the numbers/money section has to compute. I'll upload a patch without the enum. - Santeri ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112099/#review37845 ----------------------------------------------------------- On Aug. 15, 2013, 5:07 p.m., Santeri Piippo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112099/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2013, 5:07 p.m.) > > > Review request for KDE Runtime and Albert Astals Cid. > > > Description > ------- > > when the input texts are changed the sample did not update and only was > updated with the 1 second timer. with applied patch it updates right away. > > for performance reasons updateSample() was given a flags parameter for > filtering out what to update, defaults to all, i.e. original behavior. > > > Diffs > ----- > > kcontrol/locale/kcmlocale.h c7b29bc > kcontrol/locale/kcmlocale.cpp 62d5aef > > Diff: http://git.reviewboard.kde.org/r/112099/diff/ > > > Testing > ------- > > replaced kcm_locale.so in kubuntu 13.04 with git-compiled one, works fine for > me. > > > Thanks, > > Santeri Piippo > >
