- Dillo-dev - lists.mailman3.com

[patch]: take 2: FtlkListResource

by Johannes.Hofmann＠gmx.de

On Thu, May 01, 2008 at 03:09:41PM +0100, Jeremy Henty wrote: > > Since Johannes' criticisms were (IIRC) mostly about my patch's > complexity I hope he will look at a new patch that sticks to my > original approach but is a lot simpler. > > I realised that if the user_data() of the Menu Widget items stored the > index of the Item instead of the Item itself then FltkListResource > could store the selections by index in a misc::SimpleVector<bool>. > This means there is no need to wrap bools in object::Object pointers > and no need to add instance methods to Items to make them > Hashable. Most of the scaffolding of the previous patch just > disappears (the line count decreases by a third). > > I don't want to dismiss Johannes' alternative suggestion out of hand, > but I still think my approach has a lot going for it and I'd like > people to have a chance to criticise my best shot at it. Very nice. This is really much simpler. I also understand, your point about not to extend Item with members specific for this usecase. Cheers, Johannes > > Comments are welcome! > > Jeremy Henty > diff -pru -- 00_pre/dw/fltkui.cc 01_post/dw/fltkui.cc > --- 00_pre/dw/fltkui.cc 2008-04-26 21:42:58.000000000 +0100 > +++ 01_post/dw/fltkui.cc 2008-05-01 14:00:11.000000000 +0100 > @@ -858,10 +858,11 @@ template <class I> FltkSelectionResource > } > > template <class I> > -::fltk::Item *FltkSelectionResource::Item::createNewWidget () > +::fltk::Item *FltkSelectionResource::Item::createNewWidget (int index) > { > - ::fltk::Item *item = new ::fltk::Item (name); > - item->user_data (this); > + ::fltk::Item *item = > + new ::fltk::Item (name); > + item->user_data ((void *) index); > return item; > } > > @@ -869,7 +870,10 @@ template <class I> > ::fltk::ItemGroup * > FltkSelectionResource::Item::createNewGroupWidget () > { > - return new ::fltk::ItemGroup (name); > + ::fltk::ItemGroup *itemGroup = > + new ::fltk::ItemGroup (name); > + itemGroup->user_data ((void *) -1); > + return itemGroup; > } > > > @@ -922,12 +926,14 @@ FltkSelectionResource::createNewWidge > > ::fltk::Group *currGroup = widgetStack->stack->getTop()->getTypedValue(); > > + int index = 0; > for (Iterator <Item> it = allItems->iterator (); it.hasNext (); ) { > Item *item = it.getNext (); > switch (item->type) { > case Item::ITEM: > - itemWidget = item->createNewWidget (); > + itemWidget = item->createNewWidget (index); > currGroup->add (itemWidget); > + index++; > break; > > case Item::START: > @@ -960,6 +966,7 @@ template <class I> void FltkSelectionRes > bool enabled, > bool selected) > { > + int index = items->size (); > Item *item = createNewItem (Item::ITEM, str, enabled, selected); > items->put (item); > allItems->append (item); > @@ -967,7 +974,7 @@ template <class I> void FltkSelectionRes > for (Iterator <WidgetStack> it = widgetStacks->iterator (); > it.hasNext(); ) { > WidgetStack *widgetStack = it.getNext (); > - ::fltk::Item *itemWidget = item->createNewWidget (); > + ::fltk::Item *itemWidget = item->createNewWidget (index); > widgetStack->stack->getTop()->getTypedValue()->add (itemWidget); > > if (!enabled) > @@ -1031,10 +1038,10 @@ template <class I> const char *FltkSelec > // ---------------------------------------------------------------------- > > FltkOptionMenuResource::FltkOptionMenuResource (FltkPlatform *platform): > - FltkSelectionResource <dw::core::ui::OptionMenuResource> (platform) > + FltkSelectionResource <dw::core::ui::OptionMenuResource> (platform), > + selection(-1) > { > init (platform); > - selection = NULL; > } > > FltkOptionMenuResource::~FltkOptionMenuResource () > @@ -1057,7 +1064,7 @@ void FltkOptionMenuResource::widgetCallb > void *data) > { > ((FltkOptionMenuResource *) data)->selection = > - ((::fltk::Menu *) widget)->item()->user_data(); > + (int) (((::fltk::Menu *) widget)->item()->user_data()); > } > > void FltkOptionMenuResource::sizeRequest (core::Requisition *requisition) > @@ -1092,14 +1099,14 @@ void FltkOptionMenuResource::addItem (co > FltkSelectionResource<dw::core::ui::OptionMenuResource>::addItem > (str,enabled,selected); > if (selected) > - selection = items->get (items->size()-1); > + selection = (items->size ()) - 1; > > queueResize (true); > } > > bool FltkOptionMenuResource::isSelected (int index) > { > - return items->get(index) == selection; > + return index == selection; > } > > // ---------------------------------------------------------------------- > @@ -1107,7 +1114,8 @@ bool FltkOptionMenuResource::isSelected > FltkListResource::FltkListResource (FltkPlatform *platform, > core::ui::ListResource::SelectionMode > selectionMode): > - FltkSelectionResource <dw::core::ui::ListResource> (platform) > + FltkSelectionResource <dw::core::ui::ListResource> (platform), > + itemsSelected(8) > { > init (platform); > } > @@ -1119,10 +1127,34 @@ FltkListResource::~FltkListResource () > > ::fltk::Menu *FltkListResource::createNewMenu (core::Allocation *allocation) > { > - return > + ::fltk::Menu *menu = > new ::fltk::MultiBrowser (allocation->x, allocation->y, > allocation->width, > allocation->ascent + allocation->descent); > + menu->callback(widgetCallback,this); > + menu->when(::fltk::WHEN_CHANGED); > + return menu; > +} > + > +void FltkListResource::widgetCallback (::fltk::Widget *widget, void *data) > +{ > + ::fltk::Widget *fltkItem = ((::fltk::Menu *) widget)->item (); > + int index = (int) (fltkItem->user_data ()); > + if (index == -1) return; > + bool selected = fltkItem->selected (); > + ((FltkListResource *) data)->itemsSelected.set (index, selected); > +} > + > +void FltkListResource::addItem (const char *str, bool enabled, bool selected) > +{ > + FltkSelectionResource<dw::core::ui::ListResource>::addItem > + (str,enabled,selected); > + int index = itemsSelected.size (); > + itemsSelected.increase (); > + itemsSelected.set (index,selected); > + > + // blindly copied from FltkOptionMenuResource, is it necessary? > + queueResize (true); > } > > void FltkListResource::sizeRequest (core::Requisition *requisition) > @@ -1135,8 +1167,7 @@ void FltkListResource::sizeRequest (core > > bool FltkListResource::isSelected (int index) > { > - /** todo Not implemented. */ > - return true; > + return itemsSelected.get (index); > } > > } // namespace ui > diff -pru -- 00_pre/dw/fltkui.hh 01_post/dw/fltkui.hh > --- 00_pre/dw/fltkui.hh 2008-04-26 21:42:58.000000000 +0100 > +++ 01_post/dw/fltkui.hh 2008-05-01 14:00:11.000000000 +0100 > @@ -458,8 +458,8 @@ protected: > bool selected = false); > ~Item (); > > - ::fltk::Item *createNewWidget (); > - ::fltk::ItemGroup *createNewGroupWidget (); > + ::fltk::Item *createNewWidget (int index); > + ::fltk::ItemGroup *createNewGroupWidget (); > }; > > class WidgetStack: public object::Object > @@ -508,7 +508,7 @@ protected: > > private: > static void widgetCallback (::fltk::Widget *widget, void *data); > - void *selection; > + int selection; > > public: > FltkOptionMenuResource (FltkPlatform *platform); > @@ -526,11 +526,17 @@ class FltkListResource: > protected: > ::fltk::Menu *createNewMenu (core::Allocation *allocation); > > +private: > + static void widgetCallback (::fltk::Widget *widget, void *data); > + misc::SimpleVector <bool> itemsSelected; > + > public: > FltkListResource (FltkPlatform *platform, > core::ui::ListResource::SelectionMode selectionMode); > ~FltkListResource (); > > + void addItem (const char *str, bool enabled, bool selected); > + > void sizeRequest (core::Requisition *requisition); > bool isSelected (int index); > }; > diff -pru -- 00_pre/test/dw_ui_test.cc 01_post/test/dw_ui_test.cc > --- 00_pre/test/dw_ui_test.cc 2008-04-26 21:42:59.000000000 +0100 > +++ 01_post/test/dw_ui_test.cc 2008-05-01 14:00:11.000000000 +0100 > @@ -192,7 +192,7 @@ int main(int argc, char **argv) > checklabel->addText (strdup (" check"), cellStyle); > checklabel->flush (); > > - for(int i = 0; i < 1; i++) { > + for(int i = 0; i < 2; i++) { > table->addRow (cellStyle); > > Embed *sel = new Embed (selres[i]); > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

May 1, 2008

[patch]: take 2: FtlkListResource

by onepoint＠starurchin.org

Since Johannes' criticisms were (IIRC) mostly about my patch's complexity I hope he will look at a new patch that sticks to my original approach but is a lot simpler. I realised that if the user_data() of the Menu Widget items stored the index of the Item instead of the Item itself then FltkListResource could store the selections by index in a misc::SimpleVector<bool>. This means there is no need to wrap bools in object::Object pointers and no need to add instance methods to Items to make them Hashable. Most of the scaffolding of the previous patch just disappears (the line count decreases by a third). I don't want to dismiss Johannes' alternative suggestion out of hand, but I still think my approach has a lot going for it and I'd like people to have a chance to criticise my best shot at it. Comments are welcome! Jeremy Henty

May 1, 2008

[patch]: FtlkListResource select callbacks

by onepoint＠starurchin.org

On Wed, Apr 30, 2008 at 10:52:37PM +0200, Johannes Hofmann wrote: > > This is a first cut at making FltkListResource work properly. > > > > The resource maintains a hash of its items' selected status, the > > MultiBrowser widget gets a callback that updates the hash and > > isSelected() looks up the hash for the answer. It's the same > > technique I used for FltkOptionMenuResource , where the FLTK > > widgets are linked to their resources by user_data() and callback > > arguments. > > > > I added equals() and hashValue() methods to Items so they could be > > hash keys, and implemented an object::Object wrapper class for > > boolean values to be the hash values. The wrapper class is inside > > FltkListResource , though it could easily move to lout::object > > IMHO. > > It seems a bit complicated to me. Wouldn't it be easier to just add > a member "selected" to Item that stores whether the item is > currently selected or not? May well be that I'm missing something > though. It's not so much complicated as cumbersome: the logic is pretty straightforward, but using the Hash requires extra scaffolding. I was reluctant to add stuff to Item because that class serves double duty for both FltkListResource and FltkOptionMenuResource and so it's ugly for Item to contain stuff that is specific to one of the resource classes. Of course the resource classes could derive their own Item classes, but that adds scaffolding too. IIRC I experimented with that when I first did FltkOptionMenuResource (before going for a simpler suggestion of yours, I think) and the templating made it particularly messy. Let me think about it a little while I fix the bugs. I think the bug fixes will be orthogonal to the issue of how we store the selection information so it shouldn't impact implementing your suggestions if that's the way to go. Regards, Jeremy Henty

May 1, 2008

[patch]: FtlkListResource select callbacks

by Johannes.Hofmann＠gmx.de

Hi Jeremy, On Wed, Apr 30, 2008 at 09:33:36PM +0100, Jeremy Henty wrote: > > This is a first cut at making FltkListResource work properly. > > The resource maintains a hash of its items' selected status, the > MultiBrowser widget gets a callback that updates the hash and > isSelected() looks up the hash for the answer. It's the same > technique I used for FltkOptionMenuResource , where the FLTK widgets > are linked to their resources by user_data() and callback arguments. > > I added equals() and hashValue() methods to Items so they could be > hash keys, and implemented an object::Object wrapper class for boolean > values to be the hash values. The wrapper class is inside > FltkListResource , though it could easily move to lout::object IMHO. It seems a bit complicated to me. Wouldn't it be easier to just add a member "selected" to Item that stores whether the item is currently selected or not? May well be that I'm missing something though. Cheers, Johannes > > The patch also enables the ListResource widget in test/dw-ui-test . > > Unfortunately it's still a little buggy. Specifying multiple selected > items initially doesn't work right: they are all selected in the > resource but only one is selected on the FLTK widget. Things also go > out of sync when you select a group item: any previously selected item > is unselected in the widget but not in the resource. I'll look into > these further. > > Comments are welcome! > > Regards, > > Jeremy Henty > diff -pru -- 00_pre/dw/fltkui.cc 01_post/dw/fltkui.cc > --- 00_pre/dw/fltkui.cc 2008-04-26 21:42:58.000000000 +0100 > +++ 01_post/dw/fltkui.cc 2008-04-30 20:37:40.000000000 +0100 > @@ -858,6 +858,22 @@ template <class I> FltkSelectionResource > } > > template <class I> > +bool FltkSelectionResource::Item::equals (Object *other) > +{ > + return ((void *) this) == ((void *) other); > +} > + > +template <class I> > +int FltkSelectionResource::Item::hashValue () > +{ > +#if SIZEOF_VOID_P == 4 > + return (int)this; > +#else > + return ((int*) this)[0] ^ ((int*) this)[1]; > +#endif > +} > + > +template <class I> > ::fltk::Item *FltkSelectionResource::Item::createNewWidget () > { > ::fltk::Item *item = new ::fltk::Item (name); > @@ -1107,7 +1123,8 @@ bool FltkOptionMenuResource::isSelected > FltkListResource::FltkListResource (FltkPlatform *platform, > core::ui::ListResource::SelectionMode > selectionMode): > - FltkSelectionResource <dw::core::ui::ListResource> (platform) > + FltkSelectionResource <dw::core::ui::ListResource> (platform), > + itemsSelected(false,false) > { > init (platform); > } > @@ -1119,10 +1136,34 @@ FltkListResource::~FltkListResource () > > ::fltk::Menu *FltkListResource::createNewMenu (core::Allocation *allocation) > { > - return > + ::fltk::Menu *menu = > new ::fltk::MultiBrowser (allocation->x, allocation->y, > allocation->width, > allocation->ascent + allocation->descent); > + menu->callback(widgetCallback,this); > + menu->when(::fltk::WHEN_CHANGED); > + return menu; > +} > + > +void FltkListResource::widgetCallback (::fltk::Widget *widget, void *data) > +{ > + ::fltk::Widget *fltkItem = ((::fltk::Menu *) widget)->item (); > + Item *item = (Item *) (fltkItem->user_data ()); > + if (!item) return; > + Selection *selected = Selection::fromBool(fltkItem->selected ()); > + ((FltkListResource *) data)->itemsSelected.put (item, selected); > +} > + > +void FltkListResource::addItem (const char *str, bool enabled, bool selected) > +{ > + FltkSelectionResource<dw::core::ui::ListResource>::addItem > + (str,enabled,selected); > + Item *item = items->get (items->size()-1); > + Selection *is_selected = Selection::fromBool (selected); > + itemsSelected.put(item,is_selected); > + > + // blindly copied from FltkOptionMenuResource, is it necessary? > + queueResize (true); > } > > void FltkListResource::sizeRequest (core::Requisition *requisition) > @@ -1135,8 +1176,15 @@ void FltkListResource::sizeRequest (core > > bool FltkListResource::isSelected (int index) > { > - /** todo Not implemented. */ > - return true; > + return itemsSelected.get(items->get(index))->getValue(); > +} > + > +FltkListResource::Selection FltkListResource::Selection::SELECTED(true); > +FltkListResource::Selection FltkListResource::Selection::UNSELECTED(false); > + > +FltkListResource::Selection *FltkListResource::Selection::fromBool(bool value) > +{ > + return value ? &SELECTED : &UNSELECTED; > } > > } // namespace ui > diff -pru -- 00_pre/dw/fltkui.hh 01_post/dw/fltkui.hh > --- 00_pre/dw/fltkui.hh 2008-04-26 21:42:58.000000000 +0100 > +++ 01_post/dw/fltkui.hh 2008-04-30 20:37:40.000000000 +0100 > @@ -458,6 +458,9 @@ protected: > bool selected = false); > ~Item (); > > + bool equals (Object *other); > + int hashValue (); > + > ::fltk::Item *createNewWidget (); > ::fltk::ItemGroup *createNewGroupWidget (); > }; > @@ -526,11 +529,31 @@ class FltkListResource: > protected: > ::fltk::Menu *createNewMenu (core::Allocation *allocation); > > +private: > + > + class Selection : public Object { > + > + private: > + bool value; > + > + public: > + Selection(bool value) { this->value = value; } > + inline int getValue() { return value; } > + > + static Selection SELECTED, UNSELECTED; > + static Selection *fromBool (bool value); > + }; > + > + static void widgetCallback (::fltk::Widget *widget, void *data); > + container::typed::HashTable <Item, Selection> itemsSelected; > + > public: > FltkListResource (FltkPlatform *platform, > core::ui::ListResource::SelectionMode selectionMode); > ~FltkListResource (); > > + void addItem (const char *str, bool enabled, bool selected); > + > void sizeRequest (core::Requisition *requisition); > bool isSelected (int index); > }; > diff -pru -- 00_pre/test/dw_ui_test.cc 01_post/test/dw_ui_test.cc > --- 00_pre/test/dw_ui_test.cc 2008-04-26 21:42:59.000000000 +0100 > +++ 01_post/test/dw_ui_test.cc 2008-04-30 20:37:40.000000000 +0100 > @@ -192,7 +192,7 @@ int main(int argc, char **argv) > checklabel->addText (strdup (" check"), cellStyle); > checklabel->flush (); > > - for(int i = 0; i < 1; i++) { > + for(int i = 0; i < 2; i++) { > table->addRow (cellStyle); > > Embed *sel = new Embed (selres[i]); > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

April 30, 2008

dillo-fltk on x86_64

by corvid＠lavabit.com

Jorge wrote: > A patch for ~/.dillo creation was just committed. I think dillo should tell the user that it is creating the directory.

April 30, 2008

[patch]: FtlkListResource select callbacks

by onepoint＠starurchin.org

This is a first cut at making FltkListResource work properly. The resource maintains a hash of its items' selected status, the MultiBrowser widget gets a callback that updates the hash and isSelected() looks up the hash for the answer. It's the same technique I used for FltkOptionMenuResource , where the FLTK widgets are linked to their resources by user_data() and callback arguments. I added equals() and hashValue() methods to Items so they could be hash keys, and implemented an object::Object wrapper class for boolean values to be the hash values. The wrapper class is inside FltkListResource , though it could easily move to lout::object IMHO. The patch also enables the ListResource widget in test/dw-ui-test . Unfortunately it's still a little buggy. Specifying multiple selected items initially doesn't work right: they are all selected in the resource but only one is selected on the FLTK widget. Things also go out of sync when you select a group item: any previously selected item is unselected in the widget but not in the resource. I'll look into these further. Comments are welcome! Regards, Jeremy Henty

April 30, 2008

dillo-fltk on x86_64

by jcid＠dillo.org

Hi On Sun, Apr 27, 2008 at 03:46:11AM +0200, Justus Winter wrote: > [...] > >> With my three patches applied dillo compiles and runs fine, however I > >> had to create ~/.dillo and manually start dpid. Did I miss something here? > > No, I forgot porting the creation of ~/.dillo from dillo1 to dillo2! > > (as we all run dillo1 before, ~/.dillo was there :-) A patch for ~/.dillo creation was just committed. -- Cheers Jorge.-

April 30, 2008

[patch] ignore SIGPIPE

by jcid＠dillo.org

On Thu, Apr 24, 2008 at 08:10:04PM +0200, Johannes Hofmann wrote: > Hello, > > this patch avoids exits on SIGPIPE. Currently this is only observed > on DragonFly, but dillo1 had this too. Committed (with comment). -- Cheers Jorge.-

April 30, 2008

[patch] cleanup iterator fix

by jcid＠dillo.org

On Wed, Apr 30, 2008 at 05:21:55PM +0200, Johannes Hofmann wrote: > Hi, > > now I noticed, that there is a more elegant way to create an > iterator that is winded forward until its end. > This restores symmetry between DeepIterator::next() and > DeepIterator::prev(). Committed. -- Cheers Jorge.-

April 30, 2008

[patch] two more tiny memory leaks

by jcid＠dillo.org

On Wed, Apr 30, 2008 at 05:24:00PM +0200, Johannes Hofmann wrote: > Hi, > > two more small leaks in dw2 found by valgrind. Committed. -- Cheers Jorge.-

April 30, 2008