Every BOARD_ITEM has a parent/child relationship or at least it did if someone did not break it. You can always find the parent BOARD object by walking up the BOARD_ITEM parent list. There is already a BOARD_ITEM::GetBoard() call that should return the parent board. You could use that to notify the BOARD object when a child BOARD_ITEM change effects the bounding box.
On 2/24/2017 9:27 AM, Jon Evans wrote: > We need a solution that allows GetBoundingBox to be called blindly, > without knowing if (or how) an EDA_ITEM subclass needs to have the > bounding box updated. > > Does anyone have ideas about how complicated it would be to give the > BOARD class knowledge of whether or not anything inside it has changed? > This is what I tried to implement at first, but Orson pointed out that > the way I did it would miss some kinds of changes. > > Figuring out the above also seems useful for other reasons -- for > example, autosave / backup features are easier to implement if there is > a global "dirty/clean" flag for things like BOARD. > > If there is no good way to implement caching, I guess another way would > be to implement caching at the sites that use the BOARD bounding box > heavily (autorouter etc) > > -Jon > > On Fri, Feb 24, 2017 at 9:22 AM, Wayne Stambaugh <stambau...@gmail.com > <mailto:stambau...@gmail.com>> wrote: > > On 2/24/2017 4:16 AM, Maciej Sumiński wrote: > > Hi Jon, > > > > The current version looks much better to me. From what I see there is no > > actual bounding box caching, as GetBoundingBox() always calls > > ComputeBoundingBox(). I am fine with that, but before I push the patch I > > need to ask: does anyone know why the board bounding box is cached? I > > believe it must have been done to boost performance of certain parts of > > the code, but I wonder if it is really necessary. Especially that one > > needs to know that it has to be updated using ComputeBoundingBox(). > > It was cached so it didn't need to be recalculated every time the > bounding box was required. It should or at least it did recalculate > only when the changes were made since the last call to fetch the > bounding box. On very complex boards, this can take a while. Getting > the bounding box is called fairly often so this can add up. I don't see > any reason to recalculate the bounding box on every get bounding box > call. If the current bounding box behavior is broken, then it should be > fixed. I don't see any reason to get rid of the caching method. If you > are operating outside of the normal methods for adding and/or editing > board objects that prevents the correct bounding box from being > calculated, that is your fault and you need to fix your code > accordingly. > > > > > Just by looking at the code, I noticed that the autorouter calls > > BOARD::GetBoundingBox() frequently, but I could not see much > difference > > with caching disabled. Likely, the routing algorithm takes > significantly > > more time than the bounding box calculations. > > > > Personally I would completely remove m_BoundingBox field instead of > > making it mutable. Also, I have noticed there are a few places > where the > > bounding box is overridden with SetBoundingBox(). It seems to me a bit > > dubious, as bounding box should depend solely on the items > belonging to > > the board. I attach a patch to show what I would change. Any comments, > > especially from Wayne & Jean-Pierre? > > > > Regards, > > Orson > > > > On 02/23/2017 01:49 PM, Jon Evans wrote: > >> Hi Orson, > >> > >> Here's an updated patch with the changes you requested. The only > issue is, > >> without some kind of caching, I had to change the call sites that > were > >> interested in the board bounding box with edges only, so the > patch has > >> grown in scope. Let me know if this looks better. > >> > >> Best, > >> Jon > >> > >> On Thu, Feb 23, 2017 at 4:17 AM, Maciej Sumiński > <maciej.sumin...@cern.ch <mailto:maciej.sumin...@cern.ch>> > >> wrote: > >> > >>> Hi Jon, > >>> > >>> I really like the generic approach in the zoom methods. This part I > >>> would merge instantly, but there is an issue with caching the board > >>> bounding box. It does not take into account that items already > added to > >>> board may change their position and affect the bounding box. I would > >>> remove caching completely, what do you think? > >>> > >>> If you are going to modify the patch, would you rename boardBBox in > >>> COMMON_TOOLS::ZoomFitScreen() to bbox or anything that is not > related to > >>> board? Thank you in advance. > >>> > >>> Regards, > >>> Orson > >>> > >>> On 02/23/2017 05:34 AM, Jon Evans wrote: > >>>> Hi all, > >>>> > >>>> This patch finishes off the refactor I did a few days ago, by > enabling > >>>> ZoomFitScreen to work without knowledge of the BOARD class. > >>>> > >>>> In order to make this work, I changed the way GetBoundingBox and > >>>> ComputeBoundingBox work so that the call sites of > GetBoundingBox don't > >>> have > >>>> to also call ComputeBoundingBox if they don't need to use the > >>>> aBoardEdgesOnly flag. > >>>> > >>>> Those with good knowledge of BOARD should review this to make > sure I > >>> caught > >>>> all the instances where we should mark the bounding box as > needing to be > >>>> re-computed. > >>>> > >>>> Best, > >>>> Jon > >>>> > >>>> > >>>> > >>>> _______________________________________________ > >>>> Mailing list: https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > >>>> Post to : kicad-developers@lists.launchpad.net > <mailto:kicad-developers@lists.launchpad.net> > >>>> Unsubscribe : https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > >>>> More help : https://help.launchpad.net/ListHelp > <https://help.launchpad.net/ListHelp> > >>>> > >>> > >>> > >>> > >>> _______________________________________________ > >>> Mailing list: https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > >>> Post to : kicad-developers@lists.launchpad.net > <mailto:kicad-developers@lists.launchpad.net> > >>> Unsubscribe : https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > >>> More help : https://help.launchpad.net/ListHelp > <https://help.launchpad.net/ListHelp> > >>> > >>> > >> > > > > > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > > Post to : kicad-developers@lists.launchpad.net > <mailto:kicad-developers@lists.launchpad.net> > > Unsubscribe : https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > > More help : https://help.launchpad.net/ListHelp > <https://help.launchpad.net/ListHelp> > > > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > Post to : kicad-developers@lists.launchpad.net > <mailto:kicad-developers@lists.launchpad.net> > Unsubscribe : https://launchpad.net/~kicad-developers > <https://launchpad.net/~kicad-developers> > More help : https://help.launchpad.net/ListHelp > <https://help.launchpad.net/ListHelp> > > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
