Hi,
Yes, I most definitely like the new performance, clear now takes about 70ms
compared to the previous 1500ms. And it wasn't 8000 children I had, more like
20000 - I was mistaken there.
Unfortunately, I discovered a serious problem with the new solution. It seems
like in some situations, the focus updating during clear causes crashes. When a
child is deleted from Fl_Group::clear, fl_throw_focus calls fl_fix_focus which
sets off a chain of events downwards in the widget hierarchy - the event in my
case was FL_MOVE - maybe because I move the mouse during the clearing? Anyway,
finally Fl_Scroll::handle traverses children that have been deleted and that
causes a crash when calling visible on the deleted child.
Maybe we have to cheat somehow by setting children_ to 0 before the loop in
Fl_Group::clear? Or do we end up with the same bug as in previous versions then?
/Andreas
> This is a multi-part message in MIME format.
> --------------070109030502040802030704
> Content-Type: text/plain; charset=ISO-8859-15; format=flowed
> Content-Transfer-Encoding: 7bit
>
> Andreas Ekstrand wrote:
>
> > I have only measured the timing in debug mode, and my feeling of running in
> > optimized release mode is that it's even faster with the old
> > implementation, that the difference between the old and new implementation
> > feels larger there. I guess it's different how much optimization that can
> > be done with the two implementations.
> >
> >>> With Fl_Scroll::remove in Debug mode, clear took about 2400ms.
> >> With the old (1.1) or new (1.3) implementation?
> >
> > With the new 1.3 implementation. Actually, when I refer to the "old
> > implementation", I mean revision 6656 of the 1.3 branch, which is what we
> > used previously. Excuse me for any unclear statements that I have made.
> >
> > Your suggested method of using Fl_Scroll::clear is unfortunately too slow,
> > about 2500ms. I also thought it would be faster because of the reversed
> > removal order, but for some reason it isn't.
> >
> > I really appreciate that you take the time to look into this issue. But
> > since we can't see a possible quick solution for this, I have to consider
> > the option of moving back to an older 1.3 revision or even 1.2 (originally
> > chosen for the 32 bit widget coordinates). It would hurt going back there,
> > but this performance drop is too big to cope with, I'm afraid...
>
> Please wait a moment!
>
> I'm just testing a new *optimized* implementation :-)
>
> Results with 40,000 children (Fl_Buttons) in Fl_Scroll:
>
> old implementation:
> - Fl_Group::clear() about 1.9 - 2.0 sec.
> - Fl_Scroll::clear() about 1.3 sec.
>
> new implementation: about 0.03 - 0.05 sec !
>
> This new implementation will also be used in Fl_Scroll::clear()
> without a significant slowdown effect.
>
> I guess you'll like it. ;-)
>
> Would you like to test it yourself? I'll append the two methods
> to replace in this message. They should replace the complete
> old methods.
>
> *** without any guarantees and promises ***
>
> I also appended scroll.cxx (my test program). Tested with
> Windows only.
>
> Comments and test results will be appreciated !
>
> Albrecht
>
> ----- Fl_Group::clear() -----
>
> /* Implementation note:
> This is optimized and does not remove children _during_ the loop.
> However, deleting one child may also delete other children in the
> group because a child widget can destroy other children in the same
> group in its destructor or simply remove them from the parent group.
> Because of this we must walk forwards through the array of children.
> The allocated array_ of children is free'd after the loop.
> */
> void Fl_Group::clear() {
> savedfocus_ = 0;
> resizable_ = this;
> init_sizes();
> // okay, now it is safe to destroy the children:
> int i;
> for (i=0; i<children_; i++) { // delete all children
> Fl_Widget* o = child(i); // get child widget
> if (o->parent()==this) { // should always be true
> o->parent_ = 0; // reset child's parent
> delete o; // delete it
> }
> }
> // now free the array_ of children (if any)
> if (children_ > 1) {
> free((void*)array_);
> }
> array_ = 0;
> children_ = 0;
> }
>
> ----- Fl_Scroll::clear() -----
>
> void Fl_Scroll::clear() {
> remove(hscrollbar);
> remove(scrollbar);
> Fl_Group::clear();
> add(hscrollbar);
> add(scrollbar);
> }
>
> --------------070109030502040802030704
> Content-Type: text/plain;
> name="scroll.cxx"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline;
> filename="scroll.cxx"
>
> //
> // modified Fl_Scroll test program for the Fast Light Tool Kit (FLTK).
> //
>
> #include <FL/Fl.H>
> #include <FL/Fl_Double_Window.H>
> #include <FL/Fl_Scroll.H>
> #include <FL/Fl_Scrollbar.H>
> #include <FL/Fl_Button.H>
>
> #include <time.h>
> #include <sys/timeb.h>
> #include <stdio.h>
>
> #include <string.h>
> #include <stdio.h>
>
> static char *ctime ()
> {
> static char tbuf[40];
> struct timeb ctime;
> struct tm *lt;
> ftime (&ctime);
> lt = localtime (&ctime.time);
> sprintf (tbuf,"%2.2d:%2.2d:%2.2d.%2.2d",
> lt->tm_hour,
> lt->tm_min,
> lt->tm_sec,
> ctime.millitm/10);
> return tbuf;
> }
>
> Fl_Scroll* thescroll = 0;
> Fl_Scrollbar* hsb = 0;
> Fl_Scrollbar* vsb = 0;
>
> void clear_cb(Fl_Widget*, void* v) {
>
> #ifdef CLEAR_READQUEUE
> int i;
> for (i=0; i<99; i++)
> if (!Fl::readqueue()) break;
> printf ("Fl::readqueue() - flushed %d entries\n",i);
> #endif
> printf ("clear_cb: %s - started,
> children=%5d\n",ctime(),thescroll->children());
>
> #define USE_GROUP_CLEAR
> #ifdef USE_GROUP_CLEAR
> printf ("using Fl_Group::clear()\n");
> thescroll->remove(hsb);
> thescroll->remove(vsb);
> thescroll->Fl_Group::clear();
> thescroll->add(hsb);
> thescroll->add(vsb);
> #else
> printf ("using Fl_Scroll::clear()\n");
> thescroll->clear();
> #endif
> printf ("clear_cb: %s - finished,
> children=%5d\n",ctime(),thescroll->children());
> thescroll->redraw();
> }
>
> int main(int argc, char** argv) {
>
> setvbuf (stdout,NULL,_IONBF,0); // stdout unbuffered
> Fl_Window window(600,400);
> Fl_Scroll scroll(0,0,600,300);
>
> int n = 0;
> for (int y=0; y<400; y++) for (int x=0; x<100; x++) {
> char buf[20]; sprintf(buf,"%d",n++);
> Fl_Button* b = new Fl_Button(x*60,y*20,60,20);
> b->copy_label(buf);
> b->color(n%128);
> b->labelcolor(FL_WHITE);
> }
> scroll.end();
> window.resizable(scroll);
>
> thescroll = &scroll;
> hsb = &thescroll->hscrollbar;
> vsb = &thescroll->scrollbar;
>
> Fl_Button *clear = new Fl_Button (20,320,60,20,"clear");
> clear->callback(clear_cb);
>
> window.end();
> window.show(argc,argv);
> return Fl::run();
> }
>
> --------------070109030502040802030704--
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
