Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Excellent, I have just performed some additional tests, added a few more fixes and everything seems fine, no observable performance drop. Thank you Jon, your patch has been merged. Regards, Orson On 03/01/2017 10:37 PM, Wayne Stambaugh wrote: > Got it! Then I'm ok with the changes assuming they

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Got it! Then I'm ok with the changes assuming they don't break anything else. On 3/1/2017 4:28 PM, Jon Evans wrote: > The caching was only used by the auto router before. Orson's patch > implements local caching in the auto router, and other sites just > regenerate the bbox when they need to

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

The caching was only used by the auto router before. Orson's patch implements local caching in the auto router, and other sites just regenerate the bbox when they need to retrieve it, which is the same behavior as before. We can't add a caching system that didn't exist before without the board

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

I thought we were retaining the cached behavior for existing code and implementing a secondary feature to always recalculate the bounding box. It looks to me as though the cached behavior has completely been removed unless I'm missing something. On 3/1/2017 9:07 AM, Jon Evans wrote: > It was in

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

It was in this thread; I've attached them both again for convenience. The "0001-Remove-BOARD-SetBoundingBox.patch" is applied after applying the other patch. On Wed, Mar 1, 2017 at 8:59 AM, Wayne Stambaugh wrote: > On 3/1/2017 8:54 AM, Jon Evans wrote: > > Wayne, Orson's

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

On 3/1/2017 8:54 AM, Jon Evans wrote: > Wayne, Orson's latest patch is basically Option 2 but implemented > another way. Can you check it out and let us know if you are OK merging > it as-is, or if we should change the implementation to be more like what > I described? I don't remember seeing

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Wayne, Orson's latest patch is basically Option 2 but implemented another way. Can you check it out and let us know if you are OK merging it as-is, or if we should change the implementation to be more like what I described? Thanks, Jon On Wed, Mar 1, 2017 at 8:43 AM, Wayne Stambaugh

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Option 2 seems fine. It may be easier to just add an option argument to the existing GetBoundingBox() function to either return the cached bounding box or recalculate the bounding box on demand rather than create a different function for the cached vs. uncached version. On 3/1/2017 2:51 AM,

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Wayne, what do you think? If it is acceptable, then I would like to merge the patches. I should not judge my code, as I might be tempted to consider it a good solution. In case my proposal to cache the bounding box in autorouter code is not approved, then I would say #2 would do the job. Cheers,

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Update: after some more testing, I think Orson's patch on top of mine is the way to go. It pulls the computation out of the loops for autorouter, and that's the only place where the board's bounding box is used repetitively. Since the other call sites are only going to get the bounding box

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

I'm finally back to this problem now that I have some other WIP committed. As far as I can tell, there two paths to go down for this problem: 1) Implement auto-caching of the bounding box. This requires some mechanism to tell BOARD that it needs to update the cache, be it OBSERVABLE or some

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

On 2/24/2017 10:30 AM, Maciej Sumiński wrote: > Most of the operations one can do on a BOARD_ITEM potentially affects > the board bounding box. It means there might be many places where the > notifications should be added. > > Some time ago, Michael Steinberg has put in place OBSERVABLE class >

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

Most of the operations one can do on a BOARD_ITEM potentially affects the board bounding box. It means there might be many places where the notifications should be added. Some time ago, Michael Steinberg has put in place OBSERVABLE class (include/observable.h) that could facilitate the task.

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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?

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

The previous mail had incorrect patch, please check this one. On 02/24/2017 10: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

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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

Re: [Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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

[Kicad-developers] [PATCH] Move ZoomFitScreen and ZoomPreset to common

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