[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: some patches for kspread and series in general
From:       Ariya Hidayat <ariya () kde ! org>
Date:       2003-12-31 14:35:49
Message-ID: 3FF2DEC5.20702 () kde ! org
[Download RAW message or body]



> ... I simplified the decision tree, but at the
> price of more lines of code. I don't know if you agree
> with this decision, but the spreed increased on my
> machine from 17 sec. to 13 sec. for the same test.

Yes, this trick will improve the speed. We can even think to go further 
to split the linear and geometric series into two separate functions. 
This would be definitely good for clarity of the code.

> ... update(), which emits a sig_updateView...so I rebuild
> this introducing a default bool parameter of false (so
> you can still use the setNumber(double) function as
> before), which I set to false before calling the
> function. One updateView and one recalc() at the end
> takes care of everything...Now the result of my test
> was 6 sec. versus the original 17 sec. But...

My strong feeling is that adding such check is just a hack. The biggest 
problem KSpread had long time ago is the flickers. Everytime some part 
of the document change, then there's an update for the view. John 
introduced emitBeginOperation/emitEndOperation once to reduce the 
possible unnecessary repeated updates, but somehow now I feel that this 
isn't so convincing either. With emitBeginOperation/emitEndOperation, 
developers have more responsibility and if  we're not careful (like what 
I and others did several times), another problem arises: no enough 
updates, hence redraw problem (check bugs.kde.org for suchs bugs, they 
come and go and come again....).
So either we have too many or too few updates.

I'm now experimenting with another workaround. Basically every time a 
cell or a region is changed, KSpreadDoc should automatically schedule a 
delayed "updated" signal (possibly using QTimer). It should also keep a 
list/stack of the changes. Once a signal is emitted, KSpreadView can 
handle that in a slot and manage to update the view appropriately. Only 
one signal should be emitted for a series of changes (similar to 
QWidget::repaint and QWidget::update).
This trick, however, would not work well if you change 10000 cells in 
autofill/series operation because the overhead would reduce the speed 
rather than improve it.

> I think the options are :
> 
> 1. Live with the broken undo :-(
> 2. Having the user select a rectangle before starting
> (cumbersome if you want to construct long series) :-(
> 3. Ignoring merged cells and let the user do the
> clean-up if a number has been 'misplaced' (fast so..)
> :-)
> 4. Do 3 and use the time freed for checking if cell in
> the range are empty and issue a warning if they are
> not :-)

I agree with you on the 4th option.

Also one small notice: I plan on migrating the KSpreadUndo stuff into 
the KoCommand-based commands, just like KWord and KPresenter use. 
Looking how clutter KSpreadView is, putting many operations in 
KSpread::Command classes will help improve code readability, as well as 
easier check and troubleshoot for undo/redo bugs. This of course will 
take some time since I can't just rewrite all the undo classes in one 
week without making the CVS version unusable.


Best regards,

Ariya Hidayat, ariya@kde.org
http://ariya.pandu.org

_______________________________________________
koffice-devel mailing list
koffice-devel@mail.kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]