# HG changeset patch # User Diggory Hardy # Date 1253038199 -7200 # Node ID 1f9d00f392bdcb38103a652dc5a90ae48af57699 # Parent 62aa8845edd2000971735723b63f89835b8d7723 Fixed a bug where (non-resizible) widgets wouldn't get shrunk when minimal size decreases, meaning optional context menus are hiden properly now. Optimised when ServiceContentList.opCall is called, I think without breaking anything. diff -r 62aa8845edd2 -r 1f9d00f392bd codeDoc/jobs.txt --- a/codeDoc/jobs.txt Tue Sep 15 10:36:37 2009 +0200 +++ b/codeDoc/jobs.txt Tue Sep 15 20:09:59 2009 +0200 @@ -3,16 +3,17 @@ In progress: -Optional context menus don't always show/hide properly. +Investigating what happens when IServiceContent.setContent is called. + Having setContent recusively call on subWidgets is not quite right: where the addContent function was used to pass a different content, the content should not be reset by a content() call propegated from a parent. -Why is ServiceContent.opCall() called in addition to call from CollapsibleWidget? -Why is ServiceContentList.opCall() not called? Probably because it's callbacks are never called. To do (importance 0-5: 0 pointless, 1 no obvious impact now, 2 todo sometime, 3 useful, 4 important, 5 urgent): Also search for FIXME/NOTE/BUG/WARNING comment marks. +5 Context menu callbacks (actions) sometimes are sometimes called when opening the context menu. 4 Move createWidget code out of WidgetManager. +4 Make GridLayoutWidget decrease size when no cols/rows are resizable and min-size reduces (to make optional context menu items disappear completely when not available). 3 Make clickEvent only for down-clicks? 3 RequestRedraw should become a function of the renderer. 3 Implement a RootWidget moving functionality out of AWidgetManager, etc., now, or later? diff -r 62aa8845edd2 -r 1f9d00f392bd mde/content/Content.d --- a/mde/content/Content.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/content/Content.d Tue Sep 15 20:09:59 2009 +0200 @@ -126,8 +126,10 @@ * super(); * --- */ final void endEvent () { - foreach (dg; cb) + foreach (dg; cb) { + debug logger.trace ("calling callback {}", dg.ptr); dg (this); + } } override char[] toString (uint i) { diff -r 62aa8845edd2 -r 1f9d00f392bd mde/content/ServiceContent.d --- a/mde/content/ServiceContent.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/content/ServiceContent.d Tue Sep 15 20:09:59 2009 +0200 @@ -33,7 +33,13 @@ } } -/** Interface for ServiceContent and ServiceContentList. */ +/** Interface for ServiceContent and ServiceContentList. + * + * When the value changes, needs to call callbacks from collapsible widgets and + * the like, to show/hide the button. + * + * When ServiceContent buttons are pressed, they need to call event callbacks, + * doing the service. */ interface IServiceContent : IContent { void setContent (Content cont); } @@ -56,8 +62,11 @@ override void setContent (Content cont) { T oCont = activeCont; activeCont = cast(T)cont; - if ((oCont !is null) != (activeCont !is null)) + if ((oCont !is null) != (activeCont !is null)) { + logger.trace ("00"); endEvent; + logger.trace ("01"); + } } override bool opCall () { @@ -80,22 +89,42 @@ { this (char[] symbol) { super (symbol); + foreach (child; list_) { + if ((cast(IBoolContent)child)()) { + v = true; + break; + } + } + endEvent; } void setContent (Content cont) { foreach (child; list_) { (cast(IServiceContent)child).setContent (cont); } + bool ov = v; + v = false; + foreach (child; list_) { + if ((cast(IBoolContent)child)()) { + v = true; + break; + } + } + if (v != ov) { + debug logger.trace ("ServiceContentList.endEvent"); + endEvent; + } } override void append (Content x) { assert (cast(IBoolContent) x, "Only IBoolContent children are allowed!"); list_ ~= x; - x.addCallback (&childChangeCB); + //NOTE: this should only ever be changed when setContent is called and after creation + //x.addCallback (&childChangeCB); } override bool opCall () { - debug logger.trace ("ServiceContentList.opCall"); + debug logger.trace ("ServiceContentList.opCall: {}", symbol); return v; } // Doesn't support directly setting the value @@ -117,15 +146,19 @@ (new AStringService(lName~".copy")).addCallback (delegate void(IContent c) { debug assert (cast(AStringService)c); with (cast(AStringService)c) { - if (activeCont !is null) + if (activeCont !is null) { clipboard = activeCont.toString(0); + debug logger.trace ("set clipboard to \"{}\"", clipboard); + } } }); (new AStringService(lName~".paste")).addCallback (delegate void(IContent c) { debug assert (cast(AStringService)c); with (cast(AStringService)c) { - if (activeCont !is null) + if (activeCont !is null) { activeCont = clipboard; + debug logger.trace ("assigned from clipboard: \"{}\"", activeCont.toString(0)); + } } }); @@ -148,6 +181,7 @@ } private: + /+NOTE: trying a different method void childChangeCB (IContent icont) { if (v == false) { // then value changes iff icont() debug assert (cast(IBoolContent) icont); @@ -166,7 +200,7 @@ if (!v) endEvent; } - } + }+/ bool v = false; // cache value so we can see when it changes static char[] clipboard; diff -r 62aa8845edd2 -r 1f9d00f392bd mde/gui/widget/AParentWidget.d --- a/mde/gui/widget/AParentWidget.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/gui/widget/AParentWidget.d Tue Sep 15 20:09:59 2009 +0200 @@ -89,11 +89,11 @@ // Most parent widgets need to implement these, although not all // They must at a minimum make sure widget's size is at least nmw by nmh. public override void minWChange (IChildWidget widget, wdim nmw) { - if (widget.width < nmw) + if (!widget.isWSizable || widget.width < nmw) widget.setWidth (nmw, -1); } public override void minHChange (IChildWidget widget, wdim nmh) { - if (widget.height < nmh) + if (!widget.isHSizable || widget.height < nmh) widget.setHeight (nmh, -1); } diff -r 62aa8845edd2 -r 1f9d00f392bd mde/gui/widget/Ifaces.d --- a/mde/gui/widget/Ifaces.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/gui/widget/Ifaces.d Tue Sep 15 20:09:59 2009 +0200 @@ -106,7 +106,10 @@ /** Child widgets should call this on their parent if their minimal size * changes, since they cannot properly resize themselves. * - * Parents $(I must) increase their child's size if the child is too small. + * Parents $(I must) increase their child's size if the child is too small, + * and if the child is not resizable, shrink if too big (except for reasons + * like alignment within a grid). + * * Parents $(I must not) change their own size, even if they are not * sizable; they may only change their childrens' sizes if it does not * affect their own (i.e. WidgetManager and floating / popup widgets). diff -r 62aa8845edd2 -r 1f9d00f392bd mde/gui/widget/ParentContent.d --- a/mde/gui/widget/ParentContent.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/gui/widget/ParentContent.d Tue Sep 15 20:09:59 2009 +0200 @@ -208,7 +208,7 @@ mw = nmw; parent.minWChange (this, nmw); } else { // changes won't be seen, but we must follow function spec - if (widget.width < nmw) + if (!widget.isWSizable || widget.width < nmw) widget.setWidth (nmw, -1); } } @@ -217,7 +217,7 @@ mh = nmh; parent.minHChange (this, nmh); } else { - if (widget.height < nmh) + if (!widget.isHSizable || widget.height < nmh) widget.setHeight (nmh, -1); } } @@ -336,7 +336,7 @@ super.minWChange (widget, nmw); } override void minHChange (IChildWidget widget, wdim nmh) { - debug assert (widget is subWidgets[0]); + debug assert (widget is subWidgets[0]); if (display) { mh = nmh; parent.minHChange (this, nmh); @@ -386,8 +386,7 @@ void cbDisplay (IContent) { if (display == content_()) return; display = content_(); - logger.trace ("{}.cbDisplay ({})", id, display); - if (display) { + if (display) { mw = subWidgets[0].minWidth; mh = subWidgets[0].minHeight; } else { @@ -395,7 +394,7 @@ } parent.minWChange (this, mw); parent.minHChange (this, mh); - if (!display) return; + if (!display) return; // set incase parent didn't: subWidgets[0].setWidth (w, -1); subWidgets[0].setHeight (h, -1); diff -r 62aa8845edd2 -r 1f9d00f392bd mde/gui/widget/WidgetManager.d --- a/mde/gui/widget/WidgetManager.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/gui/widget/WidgetManager.d Tue Sep 15 20:09:59 2009 +0200 @@ -145,10 +145,11 @@ return; } mw = nmw; - if (w < nmw) { - childRoot.setWidth (nmw, -1); - w = nmw; - } + matchMinimalSize (); + nmw = w; // reuse to see if width is changed + w = mw > sW ? mw : sW; + if (w != nmw) + childRoot.setWidth (w, -1); childRoot.setPosition (0,0); requestRedraw; } @@ -159,10 +160,11 @@ return; } mh = nmh; - if (h < nmh) { - childRoot.setHeight (nmh, -1); - h = nmh; - } + matchMinimalSize (); + nmh = h; + h = mh > sH ? mh : sH; + if (h != nmh) + childRoot.setHeight (nmh, -1); childRoot.setPosition (0,0); requestRedraw; } @@ -372,7 +374,9 @@ matchMinimalSize (); debug (mdeWidgets) logger.trace ("Setting size and position of root widget..."); - childRoot.setWidth (w, -1); + w = mw > sW ? mw : sW; + h = mh > sH ? mh : sH; + childRoot.setWidth (w, -1); childRoot.setHeight (h, -1); childRoot.setPosition (0,0); debug (mdeWidgets) logger.trace ("Done creating root widget."); @@ -385,13 +389,19 @@ } final void wmSizeEvent (int nw, int nh) { - w = cast(wdim) nw; - h = cast(wdim) nh; + sW = cast(wdim) nw; + sH = cast(wdim) nh; matchMinimalSize; if (!childRoot) return; // if not created yet. - childRoot.setWidth (w, -1); - childRoot.setHeight (h, -1); + if (sW != w && w != mw) { + w = mw > sW ? mw : sW; + childRoot.setWidth (w, -1); + } + if (sH != h && h != mh) { + h = mh > sH ? mh : sH; + childRoot.setHeight (h, -1); + } childRoot.setPosition (0,0); debug logWidgetSize (null); } @@ -475,7 +485,10 @@ auto oUM = underMouse; underMouse = getPopupWidget (cx, cy, closePopup); if (underMouse is null) { - debug assert (childRoot.onSelf (cx, cy), "WidgetManager: childRoot doesn't cover whole area"); + // Note: I'm surprised this never fails, since clicks outside the + // window are reported. If it does, better allow underMouse to be + // null. + debug assert (childRoot.onSelf (cx, cy), "WidgetManager: click not on childRoot"); underMouse = childRoot.getWidget (cx, cy); } debug assert (oUM && underMouse, "no widget under mouse: error"); @@ -488,20 +501,15 @@ } /** If possible, the screen-interaction derived class should override to - * make sure the window is at least (mw,mh) in size. In any case, this - * method MUST make sure w >= mw and h >= mh even if the window isn't this - * big. + * make sure the window is at least (mw,mh) in size (use sW, sH to store + * the actual size). * - * A resize may not be required when this is called, however. */ + * A resize won't always be required when this is called. */ void matchMinimalSize () { - if (w < mw) { - logger.warn ("Min width for gui, {}, not met: {}", mw, w); - w = mw; - } - if (h < mh) { - logger.warn ("Min height for gui, {}, not met: {}", mh, h); - h = mh; - } + if (sW < mw) + logger.warn ("Min width for gui, {}, not met: {}", mw, sW); + if (sH < mh) + logger.warn ("Min height for gui, {}, not met: {}", mh, sH); } /// This should be overloaded to set a callback receiving keyboard input. @@ -621,6 +629,7 @@ // Dimensions and child set-up data (fit to childRoot): wdim w,h; // current widget size; should be at least (mw,mh) even if not displayable wdim mw,mh; // minimal area required by widgets + wdim sW,sH; // actual screen size; ideally equal to w,h uint setupN; // n to pass to IChildWidget.setup // IPopupParentWidget stuff for childRoot: diff -r 62aa8845edd2 -r 1f9d00f392bd mde/gui/widget/layout.d --- a/mde/gui/widget/layout.d Tue Sep 15 10:36:37 2009 +0200 +++ b/mde/gui/widget/layout.d Tue Sep 15 20:09:59 2009 +0200 @@ -638,7 +638,9 @@ /** Get the row/column of relative position l. * * returns: - * -i if in space to left of col i, or i if on col i. */ + * -i if in space to left of col i, or i if on col i. + * + * Handles l right-of-last-column fine, asserts if l < pos[0]. */ ptrdiff_t getCell (wdim l) { debug assert (width, "AlignColumns not initialized when getCell called (code error)"); ptrdiff_t i = cols - 1; // starting from right... @@ -667,18 +669,26 @@ if (nw == w) return w; wdim diff = nw - w; - if (firstSizable == -1) - diff = adjustCellSizes (diff, cols-1, -1); - else - diff = adjustCellSizes (diff, (dir == -1 ? lastSizable : firstSizable), dir); + if (diff > 0) { + if (firstSizable == -1) + diff = adjustCellSizes (diff, cols-1, -1); + else + diff = adjustCellSizes (diff, (dir == -1 ? lastSizable : firstSizable), dir); + debug assert (diff == 0); + } else { + // For decreasing, visit all cols; it's possible some have size above minimal + diff = adjustCellSizes (diff, (dir == -1 ? cols-1 : 0), dir); + debug if (diff != 0) + logger.warn ("Unable to meet target width ({}); outstanding diff: {}", nw, diff); + } genPositions; debug if (nw != w) { - logger.trace ("resizeWidth on {} to {} failed, new width: {}, diff {}, firstSizable {}, columns {}",cast(void*)this, nw,w, diff, firstSizable, cols); + logger.error ("resizeWidth on {} to {} failed, new width: {}, diff {}, firstSizable {}, columns {}",cast(void*)this, nw,w, diff, firstSizable, cols); /+ Also print column widths & positions: - logger.trace ("resizeWidth to {} failed! Column dimensions and positions:",nw); + logger.error ("resizeWidth to {} failed! Column dimensions and positions:",nw); foreach (i,w; width) - logger.trace ("\t{}\t{}", w,pos[i]);+/ + logger.error ("\t{}\t{}", w,pos[i]);+/ } return w; } @@ -688,8 +698,12 @@ * This and resizeCols are for moving dividers between cells. */ bool findResizeCols (wdim l) { resizeU = -getCell (l); // potential start for upward-resizes - if (resizeU <= 0) - return true; // not on a space between cells + if (resizeU <= 0) + return true; // not on a space between cells + if (resizeU >= cols) { // right of last column; cannot resize + resizeU = -1; + return true; + } resizeD = resizeU - 1; // potential start for downward-resizes while (!sizable[resizeU]) { // find first actually resizable column (upwards) @@ -717,12 +731,13 @@ // do shrinking first (in case we hit the minimum) if (diff >= 0) { - diff = -adjustCellSizes (-diff, resizeU, 1); - adjustCellSizes (diff, resizeD, -1); + diff = adjustCellSizes (-diff, resizeU, 1) - diff; + diff = adjustCellSizes (diff, resizeD, -1); } else { - diff = -adjustCellSizes (diff, resizeD, -1); - adjustCellSizes (diff, resizeU, 1); + diff = adjustCellSizes (diff, resizeD, -1) - diff; + diff = adjustCellSizes (diff, resizeU, 1); } + debug assert (diff == 0); genPositions; } @@ -751,24 +766,37 @@ if (minWidth[col] == nmw) // no change return false; minWidth[col] = nmw; - if (!sizable[col] && lastSizable >= 0) - nd = width[col] - nmw; // Not resizable but another column is - // Else leave larger; mustn't shrink ourself - } else - return false; - + if (!sizable[col]) + nd = width[col] - nmw; // Not resizable, so shrink + // Else resizable so leave + } mw = spacing * cast(wdim)(cols - 1); foreach (imw; minWidth) mw += imw; - if (nd != 0) { // needs enlarging or shrinking + if (nd == 0) + return false; + + if (nd < 0) { // needs enlarging; we should do so here + // set new width: width[col] = nmw; foreach (cb; cbs) cb.setWidth (col, nmw, -1); - if (lastSizable >= 0) - adjustCellSizes (nd, lastSizable, -1); - } - + // Try to compensate (keep overall size the same by shrinking larger + // than necessary columns); may not be possible: + adjustCellSizes (nd, 0, 1); + } else if (lastSizable >= 0) { + // If another column can be increased, do that (otherwise the call + // to parent.minXChange should decrease, if possible): + // Although it _may_ not, if e.g. alignment forces larger-than-minimal size. + width[col] = nmw; + foreach (cb; cbs) + cb.setWidth (col, nmw, -1); + nd = adjustCellSizes (nd, lastSizable, -1); + debug assert (nd == 0); + } + //else debug logger.trace ("expecting parent.minXChange to shrink"); + debug wdim ow = w; genPositions; debug if (w < ow) @@ -800,7 +828,8 @@ * incr = direction to resize in (added to index each step). Must be either -1 or +1. * * Returns: - * The amount adjusted. This may be larger than diff, since cellD is clamped by cellDMin. + * (diff - "the actual amount adjusted") - 0 if met exactly, <0 if unable + * to shrink as much as requested. * * Will shrink non-sizable columns if they're over minimal size. * Will increase column start, since it's assumed sizable. @@ -821,6 +850,7 @@ width[i] += diff; foreach (cb; cbs) cb.setWidth (i, width[i], incr); + return 0; } else if (diff < 0) { // decrease wdim rd = diff; // running diff @@ -843,11 +873,10 @@ i += incr; } - diff -= rd; // still had rd left to decrease (may be 0) + return rd; // still had rd left to decrease (may be 0) } // else no adjustment needed (diff == 0) - - return diff; + return 0; }