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 Comp
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 cachin
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,
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 m
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 wit
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
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?
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
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.
With
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
> (i
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 othe
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 interm
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,
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, Macie
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
wrote:
> O
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 thi
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 latest patch is basical
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 t
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
dir
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 retr
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
21 matches
Mail list logo