From 1d73c0195c8bf256f23b58d0288d1bb9eb87448d Mon Sep 17 00:00:00 2001 From: Albrecht Schlosser Date: Sat, 23 Dec 2023 18:51:43 +0100 Subject: [PATCH] Improve docs and add two new Fl_Menu_Item methods (#875) This addresses some issues pointed out by GitHub Issue #875. Documentation lacked details about Fl_Multi_Label assignment and correct memory handling. The new methods - Fl_Menu_Item::image_label(const Fl_Image *) and - Fl_Menu_Item::multi_label(const Fl_Multi_Label *) provide a cleaner interface to assign images and Fl_Multi_Label's to menu items. examples/howto-menu-with-images.cxx: carify some issues, fix leak, and use new Fl_Menu_Item::multi_label(const Fl_Multi_Label *). --- FL/Fl_Menu_Item.H | 104 +++++++++++++++++++++++++--- FL/Fl_Multi_Label.H | 81 ++++++++++++---------- examples/howto-menu-with-images.cxx | 43 ++++++++---- src/Fl_Multi_Label.cxx | 55 ++++++++++++++- 4 files changed, 219 insertions(+), 64 deletions(-) diff --git a/FL/Fl_Menu_Item.H b/FL/Fl_Menu_Item.H index 4bca0e1ce..94035d097 100644 --- a/FL/Fl_Menu_Item.H +++ b/FL/Fl_Menu_Item.H @@ -17,9 +17,10 @@ #ifndef Fl_Menu_Item_H #define Fl_Menu_Item_H -# include "Fl_Widget.H" -# include "Fl_Image.H" -# include // for FL_COMMAND and FL_CONTROL +#include +#include +#include +#include // for FL_COMMAND and FL_CONTROL // doxygen needs the following line to enable e.g. ::FL_MENU_TOGGLE to link to the enums /// @file @@ -141,19 +142,100 @@ struct FL_EXPORT Fl_Menu_Item { // methods on menu items: /** - Returns the title of the item. + Returns the title (label) of the menu item. + A NULL here indicates the end of the menu (or of a submenu). A '&' in the item will print an underscore under the next letter, - and if the menu is popped up that letter will be a "shortcut" to pick - that item. To get a real '&' put two in a row. + and if the menu is popped up that letter will be a "shortcut" to + pick that item. To get a real '&' put two in a row. + + The returned value depends on the labeltype() that has been used + to store the label. + + \returns A pointer to the label cast to (const char *) + + \retval (a) A pointer to text (const char *) + \retval (b) A pointer to an image (Fl_Image *) + \retval (c) A pointer to a "multi label" (Fl_Multi_Label *) + \retval NULL End of menu or submenu + + \see Fl_Menu_Item::label(const char *) + \see Fl_Menu_Item::label(Fl_Labeltype, const char *) + \see Fl_Menu_Item::image_label(const Fl_Image *) + \see Fl_Menu_Item::multi_label(const Fl_Multi_Label *) + \see Fl_Multi_Label::label(Fl_Menu_Item *) */ - const char* label() const {return text;} + const char* label() const { return text; } - /** See const char* Fl_Menu_Item::label() const */ - void label(const char* a) {text=a;} + /** + Sets the title (label) of the menu item. - /** See const char* Fl_Menu_Item::label() const */ - void label(Fl_Labeltype a,const char* b) {labeltype_ = a; text = b;} + \note This does \b not change the labeltype() for backwards compatibility. + Take care to assign the correct labeltype() if you assign different + label types to the same menu item sequentially. + + The default labeltype() is FL_NORMAL_LABEL. + + \see label(Fl_Labeltype, const char*) + \see const char* Fl_Menu_Item::label() const + */ + void label(const char* a) { text = a; } + + /** + Sets the title (label) and the label type of the menu item. + + The default Fl_Labeltype when using Fl_Menu_Item::label(const char* a) + is FL_NORMAL_LABEL but you can set any other label type, for instance + FL_EMBOSSED_LABEL. + + Special labeltypes are: + + - FL_ICON_LABEL: draws the icon (Fl_Image) associated with the text + - FL_IMAGE_LABEL: draws the image (Fl_Image) associated with the text + - FL_MULTI_LABEL: draws multiple parts side by side, see Fl_Multi_Label + + \see const char* Fl_Menu_Item::label() const + */ + void label(Fl_Labeltype a, const char* b) { + labeltype_ = a; + text = b; + } + + /** + Sets the title (label()) and labeltype() to an Fl_Multi_Label. + + This sets the labeltype() to \_FL_MULTI_LABEL (note the leading + underscore). + + \see const char* Fl_Menu_Item::label() const + + \since 1.4.0 + + \internal Overloading Fl_Menu_Item::image() would lead to ambiguities + when used like `item->label(0)`, hence we need our own method name. + Otherwise existing programs might not compile w/o error (e.g. fluid). + */ + void multi_label(const Fl_Multi_Label *ml) { + label(FL_MULTI_LABEL, (const char *)ml); + } + + /** + Sets the title (label()) to an icon or image. + + This sets the labeltype() to \_FL_IMAGE_LABEL (note the leading + underscore). + + \see const char* Fl_Menu_Item::label() const + + \since 1.4.0 + + \internal Overloading Fl_Menu_Item::image() would lead to ambiguities + when used like `item->label(0)`, hence we need our own method name. + Otherwise existing programs might not compile w/o error (e.g. fluid). + */ + void image_label(const Fl_Image *image) { + label(FL_IMAGE_LABEL, (const char *)image); + } /** Returns the menu item's labeltype. diff --git a/FL/Fl_Multi_Label.H b/FL/Fl_Multi_Label.H index ff46e32df..7c7f60a5a 100644 --- a/FL/Fl_Multi_Label.H +++ b/FL/Fl_Multi_Label.H @@ -1,7 +1,7 @@ // // Multi-label header file for the Fast Light Tool Kit (FLTK). // -// Copyright 1998-2015 by Bill Spitzak and others. +// Copyright 1998-2023 by Bill Spitzak and others. // // This library is free software. Distribution and use rights are outlined in // the file "COPYING" which should have been included with this file. If this @@ -22,51 +22,57 @@ struct Fl_Menu_Item; /** Allows a mixed text and/or graphics label to be applied to an Fl_Menu_Item or Fl_Widget. - Most regular FLTK widgets now support the ability to associate both images and text - with a label but some special cases, notably the non-widget Fl_Menu_Item objects, do not. - Fl_Multi_Label may be used to create menu items that have an icon and text, which would - not normally be possible for an Fl_Menu_Item. - For example, Fl_Multi_Label is used in the New->Code submenu in fluid, and others. + Most regular FLTK widgets now support the ability to associate both images and text + with a label but some special cases, notably the non-widget Fl_Menu_Item objects, do not. + Fl_Multi_Label may be used to create menu items that have an icon and text, which would + not normally be possible for an Fl_Menu_Item. + For example, Fl_Multi_Label is used in the New->Code submenu in fluid, and others. - \image html Fl_Multi_Label-menu-item.png "Menu items with icons using Fl_Multi_Label" - \image latex Fl_Multi_Label-menu-item.png "Menu items with icons using Fl_Multi_Label" width=4cm + \image html Fl_Multi_Label-menu-item.png "Menu items with icons using Fl_Multi_Label" + \image latex Fl_Multi_Label-menu-item.png "Menu items with icons using Fl_Multi_Label" width=4cm - Each Fl_Multi_Label holds two elements, labela and labelb; each may hold either a - text label (const char*) or an image (Fl_Image*). When displayed, labela is drawn first - and labelb is drawn immediately to its right. + Each Fl_Multi_Label holds two elements, \p labela and \p labelb; each may hold either a + text label (const char*) or an image (Fl_Image*). When displayed, \p labela is drawn first + and \p labelb is drawn immediately to its right. - More complex labels might be constructed by setting labelb as another Fl_Multi_Label and - thus chaining up a series of label elements. + More complex labels can be constructed by setting labelb as another Fl_Multi_Label and + thus chaining up a series of label elements. - When assigning a label element to one of labela or labelb, they should be explicitly cast - to (const char*) if they are not of that type already. + When assigning a label element to one of \p labela or \p labelb, they should be + explicitly cast to (const char*) if they are not of that type already. - Example Use: Fl_Menu_Bar - \code - Fl_Pixmap *image = new Fl_Pixmap(..); // image for menu item; any Fl_Image based widget - Fl_Menu_Bar *menu = new Fl_Menu_Bar(..); // can be any Fl_Menu_ oriented widget (Fl_Choice, Fl_Menu_Button..) + \note An Fl_Multi_Label object and all its components (label text, images, chained + Fl_Multi_Label's and their linked objects) must exist during the lifetime of the + widget or menu item they are assigned to. It is the responsibility of the user's + code to release these linked objects if necessary after the widget or menu is deleted. - // Create a menu item - int i = menu->add("File/New", ..); - Fl_Menu_Item *item = (Fl_Menu_Item*)&(menu->menu()[i]); + Example Use: Fl_Menu_Bar + \code + Fl_Pixmap *image = new Fl_Pixmap(..); // image for menu item; any Fl_Image based widget + Fl_Menu_Bar *menu = new Fl_Menu_Bar(..); // can be any Fl_Menu_ oriented widget (Fl_Choice, Fl_Menu_Button..) - // Create a multi label, assign it an image + text - Fl_Multi_Label *ml = new Fl_Multi_Label; + // Create a menu item + int i = menu->add("File/New", ..); + Fl_Menu_Item *item = (Fl_Menu_Item*)&(menu->menu()[i]); - // Left side of label is an image - ml->typea = FL_IMAGE_LABEL; - ml->labela = (const char*)image; // any Fl_Image widget: Fl_Pixmap, Fl_PNG_Image, etc.. + // Create a multi label, assign it an image + text + Fl_Multi_Label *ml = new Fl_Multi_Label; - // Right side of label is label text - ml->typeb = FL_NORMAL_LABEL; - ml->labelb = item->label(); + // Left side of label is an image + ml->typea = FL_IMAGE_LABEL; + ml->labela = (const char*)image; // any Fl_Image widget: Fl_Pixmap, Fl_PNG_Image, etc.. - // Assign the multilabel to the menu item - ml->label(item); - \endcode + // Right side of label is label text + ml->typeb = FL_NORMAL_LABEL; + ml->labelb = item->label(); - \see Fl_Label and Fl_Labeltype and examples/howto-menu-with-images.cxx - */ + // Assign the multilabel to the menu item + // ml->label(item); // deprecated since 1.4.0; backwards compatible with 1.3.x + item->label(ml); // new method since 1.4.0 + \endcode + + \see Fl_Label and Fl_Labeltype and examples/howto-menu-with-images.cxx +*/ struct FL_EXPORT Fl_Multi_Label { /** Holds the "leftmost" of the two elements in the composite label. Typically this would be assigned either a text string (const char*), @@ -87,9 +93,10 @@ struct FL_EXPORT Fl_Multi_Label { if "chaining" multiple Fl_Multi_Label elements together. */ uchar typeb; - /** This method is used to associate a Fl_Multi_Label with a Fl_Widget. */ + // This method is used to associate a Fl_Multi_Label with a Fl_Widget. void label(Fl_Widget*); - /** This method is used to associate a Fl_Multi_Label with a Fl_Menu_Item. */ + + // This method is used to associate a Fl_Multi_Label with a Fl_Menu_Item. void label(Fl_Menu_Item*); }; diff --git a/examples/howto-menu-with-images.cxx b/examples/howto-menu-with-images.cxx index e0be5cad4..a5f7e70c0 100644 --- a/examples/howto-menu-with-images.cxx +++ b/examples/howto-menu-with-images.cxx @@ -1,10 +1,8 @@ // -// vim: autoindent tabstop=2 shiftwidth=2 expandtab softtabstop=2 filetype=cpp -// -// How to use Fl_Multi_Label to make menu items with images and labels. +// How to use Fl_Multi_Label to make menu items with images and labels. // // Copyright 2017 Greg Ercolano. -// Copyright 1998-2021 by Bill Spitzak and others. +// Copyright 1998-2023 by Bill Spitzak and others. // // This library is free software. Distribution and use rights are outlined in // the file "COPYING" which should have been included with this file. If this @@ -28,6 +26,8 @@ #include #include +#include // free() + // Document icon static const char *L_document_xpm[] = { "13 11 3 1", @@ -120,14 +120,20 @@ int AddItemToMenu(Fl_Menu_ *menu, // menu to add item to // Right side of label is text ml->typeb = FL_NORMAL_LABEL; - ml->labelb = item->label(); + ml->labelb = item->label(); // transfers "ownership" to ml // Assign multilabel to item. - // Note: There are two documented ways to achieve this. Both of them - // are supported, but one of them is commented out intentionally. + // There are three documented ways to achieve this. All of them are still + // supported but two of them are deprecated (see docs and comments below). + // The recommended way is to use Fl_Menu_Item::multi_label(Fl_Multi_Label *) + // which was added in FLTK 1.4.0 and sets the correct labeltype(). + // All of these statements overwrite the old label (pointer) whose + // ownership has been "transferred" to the Fl_Multi_Label (correct). + // The old label is not released (as documented). - // item->label(FL_MULTI_LABEL, (const char *)ml); - ml->label(item); + // ml->label(item); // deprecated (1.3.x) + // item->label(FL_MULTI_LABEL, (const char *)ml); // deprecated (1.3.x) + item->multi_label(ml); // new since 1.4.0 return i; } @@ -151,29 +157,37 @@ void CreateMenuItems(Fl_Menu_* menu) { // This shows why you need Fl_Multi_Label; the item->label() // gets clobbered by the item->image() setting. // + // In the following lines 'menu->add(...)' assigns a duplicated label + // string which must be released prior to assigning an image to the label + // with 'item->image()'. + // This is "not nice" but it prevents memory leaks (see GitHub Issue #875). + int i; Fl_Menu_Item *item; i = menu->add("Images/One", 0, Menu_CB, (void*)"One"); item = (Fl_Menu_Item*)&(menu->menu()[i]); - item->image(L_document_pixmap); // note: this clobbers the item's label() + free((void*)item->label()); + item->image(L_document_pixmap); i = menu->add("Images/Two", 0, Menu_CB, (void*)"Two"); item = (Fl_Menu_Item*)&(menu->menu()[i]); + free((void*)item->label()); item->image(L_folder_pixmap); i = menu->add("Images/Three", 0, Menu_CB, (void*)"Three"); item = (Fl_Menu_Item*)&(menu->menu()[i]); + free((void*)item->label()); item->image(L_redx_pixmap); } -int main() { +int main(int argc, char **argv) { Fl_Double_Window *win = new Fl_Double_Window(400, 400, "Menu items with images"); win->tooltip("Right click on window background\nfor popup menu"); // Help message Fl_Box *box = new Fl_Box(100,100,200,200); - box->copy_label(win->tooltip()); + box->label(win->tooltip()); // no need to copy_label() because it's static box->align(FL_ALIGN_CENTER|FL_ALIGN_INSIDE); // Menu bar @@ -192,12 +206,11 @@ int main() { // TODO: Show complex labels with Fl_Multi_Label. From docs: // - // "More complex labels might be constructed by setting labelb as another + // "More complex labels can be constructed by setting labelb as another // Fl_Multi_Label and thus chaining up a series of label elements." - // win->end(); win->resizable(win); - win->show(); + win->show(argc, argv); return Fl::run(); } diff --git a/src/Fl_Multi_Label.cxx b/src/Fl_Multi_Label.cxx index 72074bc12..29bbcfe61 100644 --- a/src/Fl_Multi_Label.cxx +++ b/src/Fl_Multi_Label.cxx @@ -1,7 +1,7 @@ // // Multi-label widget for the Fast Light Tool Kit (FLTK). // -// Copyright 1998-2017 by Bill Spitzak and others. +// Copyright 1998-2023 by Bill Spitzak and others. // // This library is free software. Distribution and use rights are outlined in // the file "COPYING" which should have been included with this file. If this @@ -60,10 +60,63 @@ Fl_Labeltype fl_define_FL_MULTI_LABEL() { return _FL_MULTI_LABEL; } +/** + Associate an Fl_Multi_Label with an Fl_Widget. + + This method uses Fl_Widget::label(Fl_Labeltype, const char *) internally + to set the \e label of the widget, i.e. it stores a \e pointer to the + Fl_Multi_Label object (\e this). + An existing label that has been set using Fl_Widget::copy_label() will be + released prior to the assignment of the new label. + + This sets the type of the widget's label to \_FL_MULTI_LABEL - note the + leading underscore ('_'). + + There is no way to use a method like Fl_Widget::copy_label() that transfers + ownership of the Fl_Multi_Label and its linked objects (images, text, and + chained Fl_Multi_Label's) to the widget. + + The Fl_Multi_Label and all linked images, text labels, or chained Fl_Multi_Label + objects must exist during the lifetime of the widget and will not be released + when the widget is destroyed. + + \note The user's code is responsible for releasing the Fl_Multi_Label and + all linked objects (images, text, chained Fl_Multi_Label's) after the + widget has been deleted. This may cause memory leaks if Fl_Multi_Label + is used and reassigned w/o releasing the objects assigned to it. +*/ void Fl_Multi_Label::label(Fl_Widget* o) { o->label(FL_MULTI_LABEL, (const char*)this); // calls fl_define_FL_MULTI_LABEL() } +/** + Associate an Fl_Multi_Label with an Fl_Menu_Item. + + This uses Fl_Menu_Item::label(Fl_Labeltype a, const char *b) internally to + set the \e label and the label type of the menu item, i.e. it stores a + \e pointer to the Fl_Multi_Label object (\e this). + An existing label (pointer) will be overwritten. + + This sets the type of the menu item's label to \_FL_MULTI_LABEL - note the + leading underscore ('_'). + + There is no way to use a method like Fl_Widget::copy_label() that transfers + ownership of the Fl_Multi_Label and its linked objects (images, text, and + chained Fl_Multi_Label's) to the menu item. + + The Fl_Multi_Label and all linked images, text labels, or chained + Fl_Multi_Label objects must exist during the lifetime of the menu and + will not be released when the menu item is destroyed. + + \note The user's code is responsible for releasing the Fl_Multi_Label and + all linked objects (images, text, chained Fl_Multi_Label's) after the + menu has been deleted. This may cause memory leaks if Fl_Multi_Label + is used and reassigned w/o releasing the objects assigned to it. + + \deprecated since 1.4.0: please use Fl_Menu_Item::label(Fl_Multi_Label *) + + \see Fl_Menu_Item::label(Fl_Multi_Label *) +*/ void Fl_Multi_Label::label(Fl_Menu_Item* o) { o->label(FL_MULTI_LABEL, (const char*)this); // calls fl_define_FL_MULTI_LABEL() }