Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
@manolo: Great -- agree with all, except perhaps loosing const here: - const Fl_Window *win = (const Fl_Window*)this; + Fl_Widget *win = (Fl_Widget*)this; I think we can maintain const protection on the variable this way: const Fl_Widget *win = this; : return ((Fl_Widget*)win)-as_window(); Does that not throw a warning (or even an error) about casting away the const-ness of win, though? Without actually trying it, I imagined that it might... Maybe that's what Manolo was hoping to avoid? ..this ensures const protection for all uses of win within the code, and we only turn it off when we need to (eg. the non-const as_window() call) That certainly sounds like a worthy intent, if it does not trigger any compile-time issues. I suppose we could do a more C++ish cast instead, so the compiler would know we really meant it! (i.e. const_cast) Also, I should probably change all instances of 'win' - 'w' in the code, since it's really working with widgets now. Yes, I guess that would be right. Thanks for the peer review -- will hold on comments regarding const above, as am curious if there's a reason not to. I think we should go for it... Selex ES Ltd Registered Office: Sigma House, Christopher Martin Road, Basildon, Essex SS14 3EL A company registered in England Wales. Company no. 02426132 This email and any attachments are confidential to the intended recipient and may also be privileged. If you are not the intended recipient please delete it from your system and notify the sender. You should not copy it or use it for any purpose nor disclose or distribute its contents to any other person. ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) +1 for renaming to w instead of win (or maybe widget to avoid name clashes with w(), but that's not important here, IMO) +0 for maintaining constness (yes, I mean + zero). constness doesn't gain much here, and since compilers become pickier more and more, there is the danger of getting a warning when it's cast away for as_window(), as Ian wrote elsewhere. If not now, then maybe later. We're only calling const methods anyway, as x(), y(), window(), and this code is not subject to be changed in the future. Using const here would IMHO only protect ourselves from adding code that would modify the widget in question, and as said before, we won't add more code. Just my 2 (â¬)ct. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) @Greg: Yes to all proposed changes. There's no reason, but laziness, to loose const protection. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) Albrecht We're only calling const methods anyway, as x(), y(), window(), Albrecht and this code is not subject to be changed in the future. Yes, though for some reason as_window() is not const, (it probably should be), which is why that cheat is necessary at the end there. Maybe using const_castFl_Widget*(w)-as_window() is better, which is what I decided to go with in top_window_offset().. will check. Note in manolo's implementation, const is cast away at the top at the declaration of the variable, but I just switched it to const and cast const away only at the place where it was needed, at the as_window() call. So this does two things: 1) Keeps the variable const so one can't later add code like w-label(yoo hoo); without getting an error.. hey, it could be more subtle ;) 2) Highlights exactly where the cheat is needed (at as_window()) so if as_window() ever changes in the future to be const, it's easier to find and remove the const cast-away. Is there a reason as_window() and friends (other virtual implementations) are not const methods? Fl_Window::window() is. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) Updated fixes in r9875 and r9876. About to close, unless there's more to add. Regarding const mods to at_window() and friends, let's continue that on fltk.dev and make a separate STR for that. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) Oh, I should add.. all mods to date (r9876, including static_cast) build OK on: redhat9, irix6.5, centos 5.6, Mountain Lion default compiler (picky), Sci Linux 6 (very picky, latest linux I have) gcc 4.4.6, and VS 2010. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) Sorry, as Ian points out on fltk.dev, meant to say const_cast in the above, not static_cast. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9876) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) Thanks Greg for implementing exactly what I was getting at. I still have a few remarks about the current implementation of: Fl_Window* Fl_Widget::top_window_offset(int, int) - The doc should say 'current widget' instead of 'current window' because a widget is now current. - I feel the widget-to-window cast const Fl_Window *win = (const Fl_Window *)this; is frightening, because widgets are not windows, in general. It works here because only member functions from the parent Fl_Widget class are employed. Rewriting it Fl_Widget *win = (Fl_Widget*)this; does just the same, removing the fear. - This implementation applied to a window-less widget returns this widget cast as a window, a potentially dangerous thing. My suggestion would be to use (again) our friend Fl_Widget::as_window(). The attached window.patch gathers all of this. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) Attached file window.patch... Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871)Index: src/Fl_Window.cxx === --- src/Fl_Window.cxx (revision 9873) +++ src/Fl_Window.cxx (working copy) @@ -107,19 +107,19 @@ } /** - Finds the x/y offset of the current window relative to the top-level window. + Finds the x/y offset of the current widget relative to the top-level window. \param[out] xoff,yoff Returns the x/y offset - \returns the top-level window + \returns the top-level window (or NULL for a widget that's not in any window) */ Fl_Window* Fl_Widget::top_window_offset(int xoff, int yoff) const { xoff = yoff = 0; - const Fl_Window *win = (const Fl_Window*)this; + Fl_Widget *win = (Fl_Widget*)this; while (win win-window()) { xoff += win-x(); // accumulate offsets yoff += win-y(); win = win-window(); // walk up window hierarchy } - return (Fl_Window*)win; + return win-as_window(); } /** Gets the x position of the window on the screen */ ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) @manolo: Great -- agree with all, except perhaps loosing const here: - const Fl_Window *win = (const Fl_Window*)this; + Fl_Widget *win = (Fl_Widget*)this; I think we can maintain const protection on the variable this way: const Fl_Widget *win = this; : return ((Fl_Widget*)win)-as_window(); ..this ensures const protection for all uses of win within the code, and we only turn it off when we need to (eg. the non-const as_window() call) Also, I should probably change all instances of 'win' - 'w' in the code, since it's really working with widgets now. Thanks for the peer review -- will hold on comments regarding const above, as am curious if there's a reason not to. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature - The last statement of the Fl_Widget::top_window() function may be written more simply: return w-as_window(); - I'm unsure this function would be useful. Can the knowledge of the top enclosing window of an object be useful without knowledge of the object coordinates relatively to this window? May be what is needed would be along this line: Fl_Window* Fl_Widget::top_window_offset(int xoff, int yoff) Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Can the knowledge of the top enclosing window of an object be useful without knowledge of the object coordinates relatively to this window? Simple case would be a widget (say a button) that wants to hide the window its in. If the button is in a subwindow, window() wouldn't work, bot top_window() would. May be what is needed would be along this line: Fl_Window* Fl_Widget::top_window_offset(int xoff, int yoff) Actually just added such a thing in r9866 a few days ago to solve STR#2944, but called it window_offset(). Perhaps I should add top_ to it though.. its not too late. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Patch looks good, except what Manolo wrote already. We should remove this old and error-prone w-type() = FL_WINDOW from the entire lib in favor of as_window(). WRT window_offset() vs. top_window_offset(): I strongly vote for the latter and consistent naming, i.e. top_window() and top_window_offset(). +1 for the patch with mods as written. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature as_window() Yes, that looks good.. I'll do some tests; want to make sure it inherits through all the window options we have (Fl_Gl_Window, etc) window_offset() vs. top_window_offset(): I strongly vote for the latter and consistent naming Agreed. Will make changes and implement. We should remove this old and error-prone w-type() = FL_WINDOW from the entire lib in favor of as_window(). OK, I'll try to handle that too, but as a separate commit. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Oh, BTW, I think I missed what Manolo might have been getting at wrt the 'top_window_offset()' method. So where we're at now, just to disambiguate: o There's currently a method Fl_Window::top_window_offset() (just renamed it from window_offset() in r9870) o We're proposing in this STR a new Fl_Widget::top_window(). I can see where the existing top_window_offset() should really be a method of Fl_Widget, not just Fl_Window.. so that even non-windows can find their offset relative to the top window to be more useful. So will look into changing that as well. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature OK: r9871: top_window(): implemented (using as_window()) r9872: top_window_offset() now a member of Fl_Widget (was Fl_Window) and moved it near top_window() and window(). Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ..and finally: r9873: CHANGES file updated. Comments before close? Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) Comments? Yes: Great, thanks. BTW: good catch that top_window_offset() should be a Fl_Widget method. I didn't realize that it was Fl_Window:: before. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Fix Version: 1.3-current (r9871) ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
[fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Active] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature This STR spawned from a discussion on fltk.development, Subject: RFC: method to find top level window? The suggestion is to add a method that returns the top-level window for the current widget, i.e. the 'window manager window' the widget is in. This differs from Fl_Widget::window() which may return a sub-window, if there is one, not necessarily the top-level window. I suggested toplevel_window() because the term 'top-level' seems to be used in the code comments, Albrecht offered top_window() as a shorter alternative, which I think I'll go with here. Attaching a patch suggesting the code implementation, toplevel_window.patch. Also attaching a test program, test-top_window.cxx which compares the return value of window() and top_window() for various widgets and windows in the presence of sub-windows. We should also probably do a code check on all of FLTK to make sure window() isn't being used where top_window() would be correct. I believe it's only recently that FLTK reliably supported sub-windows, so the chances of apps using them are higher now, and little problems are more likely to spring up, such as STR #2944 [2]. Comments welcome. Link: http://www.fltk.org/str.php?L2948 Version: 1.3-featureIndex: FL/Fl_Widget.H === --- FL/Fl_Widget.H (revision 9868) +++ FL/Fl_Widget.H (working copy) @@ -919,12 +919,8 @@ */ void measure_label(int ww, int hh) const {label_.measure(ww, hh);} - /** Returns a pointer to the primary Fl_Window widget. - \retval NULL if no window is associated with this widget. - \note for an Fl_Window widget, this returns its Iparent/I window -(if any), not Ithis/I window. - */ Fl_Window* window() const ; + Fl_Window* top_window() const; /** Returns an Fl_Group pointer if this widget is an Fl_Group. Index: src/Fl_Window.cxx === --- src/Fl_Window.cxx (revision 9868) +++ src/Fl_Window.cxx (working copy) @@ -80,11 +80,33 @@ clear_visible(); } +/** Returns a pointer to the nearest parent window up the widget hierarchy. +This will return sub-windows if there are any, or the parent window if there's no sub-windows. +If this widget IS the top-level window, NULL is returned. +\retval NULL if no window is associated with this widget. +\note for an Fl_Window widget, this returns its Iparent/I window + (if any), not Ithis/I window. +\see top_window() +*/ Fl_Window *Fl_Widget::window() const { for (Fl_Widget *o = parent(); o; o = o-parent()) if (o-type() = FL_WINDOW) return (Fl_Window*)o; return 0; } + +/** Returns a pointer to the top-level window for the widget. +In other words, the 'window manager window' that contains this widget. +This method differs from window() in that it won't return sub-windows (if there are any). +\returns the top-level window, or NULL if no top-level window is associated with this widget. +\see window() +*/ +Fl_Window *Fl_Widget::top_window() const { + const Fl_Widget *w = this; + while (w-parent()) { w = w-parent(); } // walk up the widget hierarchy to top-level item + return w-type() = FL_WINDOW ? ((Fl_Window*)w) // is top item a window? If so, return it.. +: ((Fl_Window*)0); // if not, return 0 +} + /** Gets the x position of the window on the screen */ int Fl_Window::x_root() const { Fl_Window *p = window(); ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev
Re: [fltk.development] [RFE] STR #2948: Add a method to Fl_Widget that returns its top-level window
DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW. [STR Pending] Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature Attached file test-top_window.cxx... Link: http://www.fltk.org/str.php?L2948 Version: 1.3-feature#include FL/Fl.H #include FL/Fl_Double_Window.H #include FL/Fl_Window.H #include FL/Fl_Button.H int main(int argc, char **argv) { Fl_Double_Window win1(900, 400, Top-level Window); win1.box(FL_FLAT_BOX); win1.color(FL_RED); Fl_Buttonb1(10,10,140,25,Button1:in top); Fl_Windowsub1(10,40,win1.w()-20,win1.h()-40-10,Sub Window); sub1.box(FL_FLAT_BOX); sub1.color(FL_GREEN); Fl_Buttonb2(10,10,140,25,Button2:in sub1); Fl_Windowsub2(10,40,sub1.w()-20,sub1.h()-40-10,Sub2 Window); sub2.box(FL_FLAT_BOX); sub2.color(FL_BLUE); Fl_Buttonb3(10,10,140,25,Button3:in sub2); sub2.end(); sub1.end(); win1.end(); win1.show(); Fl_Widget *w; w = win1; printf( WIN: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); w = b1; printf( B1: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); w = sub1; printf(SUB1: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); w = b2; printf( B2: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); w = sub2; printf(SUB2: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); w = b3; printf( B3: top_window()=%p/%-20s window()=%p/%s \n,w-top_window(), w-top_window()-label(),w-window(), w-window()?w-window()-label():?); return(Fl::run()); } // // End of $Id: table-simple.cxx 9086 2011-09-29 21:10:59Z greg.ercolano $. // ___ fltk-dev mailing list fltk-dev@easysw.com http://lists.easysw.com/mailman/listinfo/fltk-dev