Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Seth Hillbrand]

On 2019-06-09 10:29, Reece R. Pollack wrote:

Sorry about the delay in responding. I've been traveling.

I don't believe overloading the base instance of GetMsgPanelInfo() and
GetSelectMenuText() will achieve the desired goal. Overloading
functions works great when the _caller_ wants to choose between
variants of a function. In this case, though, the call is being made
via a pointer to a EDA_ITEM (in EDA_DRAW_FRAME::SetMsgPanel(),
currently at common/legacy_gal/eda_draw_frame.cpp:582). The caller has
no knowledge of whether the object requires the third parameter or
not. Since EDA_DRAW_FRAME is common to all applications, it's an all
or none situation.


Hi Reece-

The calls toe SetMsgPanel() come from the application.  You can use a 
default parameter here (and in UpdateMsgPanel()) to avoid touching the 
non-pcbnew applications with the patchset.


-Seth

___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Reece R. Pollack]

Sorry about the delay in responding. I've been traveling.

I don't believe overloading the base instance of GetMsgPanelInfo() and 
GetSelectMenuText() will achieve the desired goal. Overloading functions 
works great when the /caller/ wants to choose between variants of a 
function. In this case, though, the call is being made via a pointer to 
a EDA_ITEM (in EDA_DRAW_FRAME::SetMsgPanel(), currently at 
common/legacy_gal/eda_draw_frame.cpp:582). The caller has no knowledge 
of whether the object requires the third parameter or not. Since 
EDA_DRAW_FRAME is common to all applications, it's an all or none situation.


Realistically, I expect support for origin transforms will eventually be 
integrated into all of the applications. Pcbnew first, but users are 
going to expect consistency from Gerbview. The footprint editor would 
also benefit greatly, and thus cvpcb too. Okay, maybe the PCB Calculator 
won't support display origin transforms, and it seems the work sheet 
editor already has a similar concept.


That leaves Eeschema and the symbol editor. The Aux Origin is defined in 
Eeschema but there's no way to set it and I'm not sure that's the best 
way to handle the situation anyway. Although my previous patch set 
didn't tweak GetMsgPanelInfo() and GetSelectMenuText() in Eeschema code 
to perform display origin transforms, that could have be done easily 
since they already receive a reference to an ORIGIN_TRANSFORMS object, 
which is currently NullOriginTransforms. Then whatever scheme was 
developed for Eeschema would only require that flavor of 
ORIGIN_TRANSFORMS to be passed in and these would work. Only the 
Eeschema flavor of UNIT_BINDER and the appropriate changes to the 
dialogs would then be needed.


Also consider that much of the DRC marker code is shared between Pcbnew 
and Eeschema. While Eeschema can simply pass a reference to the 
NullObjectTransforms object to the common DRC code, this means Eeschema 
can't be completely isolated from these changes.


The alternative is for someone to come up with a scheme by which the 
editor hierarchy can provide display formatting functions to the 
structural hierarchy without passing a parameter to these 
display-oriented functions. I don't think that's likely, though I'm open 
to suggestions.


Perhaps the display formatting could be bundled into a class, rather 
than being classless, global functions as they are now. The editors 
could then pass a reference to this formatting class in place of the 
current EDA_UNITS parameter. The display formatting class could handle 
display units (mm vs in) and origin transformations internally. This 
could also help ease future formatting enhancements. However, this sort 
of change would be far broader in scope than my current patch set, and I 
was hoping for my entrance to the KiCad development community to be a 
bit less dramatic.


-Reece


On 5/26/19 5:57 PM, Wayne Stambaugh wrote:

Seth,

I'm fine with this approach.  I like your idea of deriving a new
UNIT_BINDER object so that it doesn't impact the other frames that do
not use ORIGIN_TRANSFORMS.

Cheers,

Wayne


On 5/26/19 4:15 PM, Seth Hillbrand wrote:

Hi All-

If Reece can separate this so that it doesn't affect eeschema, cvpcb,
pagelayout_editor, gerbview or libedit, I'm fine with merging as is and
refactoring more later.  If we merge this as is, it become much more
difficult to fix it later.

Since these are all GetMsgPanelInfo() calls, the fix is simply to
overload the base instance and then any call that doesn't use
ORIGIN_TRANSFORMS should not pass it as a parameter.

Once this change is implemented, the rebasing should not be nearly so
painful, so the extra holds on commits should not be required.

-Seth

The fix for this is simply overloading

On 2019-05-26 15:54, Jon Evans wrote:

Hi Reece, Wayne, Seth,

Can you clarify the path forward here?  Are we going to merge the
second patchset and then do follow-ons to take care of the issues Seth
raised, or will there be another full patchset coming?
I have a backlog of things to cherry-pick from 5.1 to master that I've
been holding on to until this is resolved.

Thanks,

-Jon

On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack 
wrote:


So now it occurs to me that what I should have done was create
classes derived from UNIT_BINDER that handle the different types of
data (X-abs, Y-abs, X-rel, Y-rel) and instantiated those, rather
than adding a parameter to the UNIT_BINDER class.

However, that would have also required changing UNIT_BINDER to
accept and return _double_ values rather than _int_ to avoid the
_int_ -> _double_ -> _int_ -> _double_ conversion sequence
formatting for display, and _double_ -> _int_ -> _double_ -> _int_
conversions parsing from display.

Maybe I'll do that another time. Too many changes too late for this
iteration, I think.

On 5/26/19 11:01 AM, Reece Pollack wrote:


The specific problem with UNIT_BINDER is that it doesn't know what
sort of data it's handling. It could be a 

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Wayne Stambaugh]

Seth,

I'm fine with this approach.  I like your idea of deriving a new
UNIT_BINDER object so that it doesn't impact the other frames that do
not use ORIGIN_TRANSFORMS.

Cheers,

Wayne


On 5/26/19 4:15 PM, Seth Hillbrand wrote:
> Hi All-
> 
> If Reece can separate this so that it doesn't affect eeschema, cvpcb,
> pagelayout_editor, gerbview or libedit, I'm fine with merging as is and
> refactoring more later.  If we merge this as is, it become much more
> difficult to fix it later.
> 
> Since these are all GetMsgPanelInfo() calls, the fix is simply to
> overload the base instance and then any call that doesn't use
> ORIGIN_TRANSFORMS should not pass it as a parameter.
> 
> Once this change is implemented, the rebasing should not be nearly so
> painful, so the extra holds on commits should not be required.
> 
> -Seth
> 
> The fix for this is simply overloading
> 
> On 2019-05-26 15:54, Jon Evans wrote:
>> Hi Reece, Wayne, Seth,
>>
>> Can you clarify the path forward here?  Are we going to merge the
>> second patchset and then do follow-ons to take care of the issues Seth
>> raised, or will there be another full patchset coming?
>> I have a backlog of things to cherry-pick from 5.1 to master that I've
>> been holding on to until this is resolved.
>>
>> Thanks,
>>
>> -Jon
>>
>> On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack 
>> wrote:
>>
>>> So now it occurs to me that what I should have done was create
>>> classes derived from UNIT_BINDER that handle the different types of
>>> data (X-abs, Y-abs, X-rel, Y-rel) and instantiated those, rather
>>> than adding a parameter to the UNIT_BINDER class.
>>>
>>> However, that would have also required changing UNIT_BINDER to
>>> accept and return _double_ values rather than _int_ to avoid the
>>> _int_ -> _double_ -> _int_ -> _double_ conversion sequence
>>> formatting for display, and _double_ -> _int_ -> _double_ -> _int_
>>> conversions parsing from display.
>>>
>>> Maybe I'll do that another time. Too many changes too late for this
>>> iteration, I think.
>>>
>>> On 5/26/19 11:01 AM, Reece Pollack wrote:
>>>
 The specific problem with UNIT_BINDER is that it doesn't know what
 sort of data it's handling. It could be a value like a track width
 which shouldn't be altered,  a relative coordinate delta which may
 need a sign flip, or an absolute coordinate which needs both
 translation and sign flip. Nor does it know whether relative or
 absolute coordinate is an X or a Y axis. Thus it must either have
 a parameter identifying the type of data it's handling or a
 different set of methods to transfer that data in or out based on
 the type. I chose a constructor parameter, and I chose to pass an
 object implemented to handle that type of data.
>>>
>>> ___
>>> 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
>> ___
>> 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
> 
> ___
> 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

___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Wayne Stambaugh]

Jon,

I've asked Reece to make Seth's recommended change to lower the merge
impact so it will probably be a while until thus is completed.  Go ahead
and cherry-pick the backlog from 5.1.

Thanks,

Wayne

On 5/26/19 3:54 PM, Jon Evans wrote:
> Hi Reece, Wayne, Seth,
> 
> Can you clarify the path forward here?  Are we going to merge the second
> patchset and then do follow-ons to take care of the issues Seth raised,
> or will there be another full patchset coming?
> I have a backlog of things to cherry-pick from 5.1 to master that I've
> been holding on to until this is resolved.
> 
> Thanks,
> -Jon
> 
> On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack  > wrote:
> 
> So _now_ it occurs to me that what I should have done was create
> classes derived from UNIT_BINDER that handle the different types of
> data (X-abs, Y-abs, X-rel, Y-rel) and instantiated those, rather
> than adding a parameter to the UNIT_BINDER class.
> 
> However, that would have also required changing UNIT_BINDER to
> accept and return /double/ values rather than /int/ to avoid the
> /int/ -> /double/ -> /int/ -> /double/ conversion sequence
> formatting for display, and /double/ -> /int/ -> /double/ -> /int/
> conversions parsing from display.
> 
> Maybe I'll do that another time. Too many changes too late for this
> iteration, I think.
> 
> On 5/26/19 11:01 AM, Reece Pollack wrote:
>> The specific problem with UNIT_BINDER is that it doesn't know what
>> sort of data it's handling. It could be a value like a track width
>> which shouldn't be altered,  a relative coordinate delta which may
>> need a sign flip, or an absolute coordinate which needs both
>> translation and sign flip. Nor does it know whether relative or
>> absolute coordinate is an X or a Y axis. Thus it must either have
>> a parameter identifying the type of data it's handling or a
>> different set of methods to transfer that data in or out based on
>> the type. I chose a constructor parameter, and I chose to pass an
>> object implemented to handle that type of data.
>>
> 
> ___
> 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
> 
> 
> ___
> 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
> 

___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Seth Hillbrand]

Hi All-

If Reece can separate this so that it doesn't affect eeschema, cvpcb, 
pagelayout_editor, gerbview or libedit, I'm fine with merging as is and 
refactoring more later.  If we merge this as is, it become much more 
difficult to fix it later.


Since these are all GetMsgPanelInfo() calls, the fix is simply to 
overload the base instance and then any call that doesn't use 
ORIGIN_TRANSFORMS should not pass it as a parameter.


Once this change is implemented, the rebasing should not be nearly so 
painful, so the extra holds on commits should not be required.


-Seth

The fix for this is simply overloading

On 2019-05-26 15:54, Jon Evans wrote:

Hi Reece, Wayne, Seth,

Can you clarify the path forward here?  Are we going to merge the
second patchset and then do follow-ons to take care of the issues Seth
raised, or will there be another full patchset coming?
I have a backlog of things to cherry-pick from 5.1 to master that I've
been holding on to until this is resolved.

Thanks,

-Jon

On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack 
wrote:


So now it occurs to me that what I should have done was create
classes derived from UNIT_BINDER that handle the different types of
data (X-abs, Y-abs, X-rel, Y-rel) and instantiated those, rather
than adding a parameter to the UNIT_BINDER class.

However, that would have also required changing UNIT_BINDER to
accept and return _double_ values rather than _int_ to avoid the
_int_ -> _double_ -> _int_ -> _double_ conversion sequence
formatting for display, and _double_ -> _int_ -> _double_ -> _int_
conversions parsing from display.

Maybe I'll do that another time. Too many changes too late for this
iteration, I think.

On 5/26/19 11:01 AM, Reece Pollack wrote:


The specific problem with UNIT_BINDER is that it doesn't know what
sort of data it's handling. It could be a value like a track width
which shouldn't be altered,  a relative coordinate delta which may
need a sign flip, or an absolute coordinate which needs both
translation and sign flip. Nor does it know whether relative or
absolute coordinate is an X or a Y axis. Thus it must either have
a parameter identifying the type of data it's handling or a
different set of methods to transfer that data in or out based on
the type. I chose a constructor parameter, and I chose to pass an
object implemented to handle that type of data.


___
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

___
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


___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Jon Evans]

Hi Reece, Wayne, Seth,

Can you clarify the path forward here?  Are we going to merge the second
patchset and then do follow-ons to take care of the issues Seth raised, or
will there be another full patchset coming?
I have a backlog of things to cherry-pick from 5.1 to master that I've been
holding on to until this is resolved.

Thanks,
-Jon

On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack  wrote:

> So *now* it occurs to me that what I should have done was create classes
> derived from UNIT_BINDER that handle the different types of data (X-abs,
> Y-abs, X-rel, Y-rel) and instantiated those, rather than adding a parameter
> to the UNIT_BINDER class.
>
> However, that would have also required changing UNIT_BINDER to accept and
> return *double* values rather than *int* to avoid the *int* -> *double*
> -> *int* -> *double* conversion sequence formatting for display, and
> *double* -> *int* -> *double* -> *int* conversions parsing from display.
>
> Maybe I'll do that another time. Too many changes too late for this
> iteration, I think.
>
> On 5/26/19 11:01 AM, Reece Pollack wrote:
>
> The specific problem with UNIT_BINDER is that it doesn't know what sort of
> data it's handling. It could be a value like a track width which shouldn't
> be altered,  a relative coordinate delta which may need a sign flip, or an
> absolute coordinate which needs both translation and sign flip. Nor does it
> know whether relative or absolute coordinate is an X or a Y axis. Thus it
> must either have a parameter identifying the type of data it's handling or
> a different set of methods to transfer that data in or out based on the
> type. I chose a constructor parameter, and I chose to pass an object
> implemented to handle that type of data.
>
>
> ___
> 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
>
___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Reece R. Pollack]

So _now_ it occurs to me that what I should have done was create classes 
derived from UNIT_BINDER that handle the different types of data (X-abs, 
Y-abs, X-rel, Y-rel) and instantiated those, rather than adding a 
parameter to the UNIT_BINDER class.


However, that would have also required changing UNIT_BINDER to accept 
and return /double/ values rather than /int/ to avoid the /int/ -> 
/double/ -> /int/ -> /double/ conversion sequence formatting for 
display, and /double/ -> /int/ -> /double/ -> /int/ conversions parsing 
from display.


Maybe I'll do that another time. Too many changes too late for this 
iteration, I think.


On 5/26/19 11:01 AM, Reece Pollack wrote:
The specific problem with UNIT_BINDER is that it doesn't know what 
sort of data it's handling. It could be a value like a track width 
which shouldn't be altered,  a relative coordinate delta which may 
need a sign flip, or an absolute coordinate which needs both 
translation and sign flip. Nor does it know whether relative or 
absolute coordinate is an X or a Y axis. Thus it must either have a 
parameter identifying the type of data it's handling or a different 
set of methods to transfer that data in or out based on the type. I 
chose a constructor parameter, and I chose to pass an object 
implemented to handle that type of data.




___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Reece Pollack]

- On May 26, 2019, at 10:53 AM, Seth Hillbrand  wrote: 

> Hi Reece-

> Apologies for the double-tap here. I see I miss-read your intention in
> a couple of places! Sorry about that. I'll try to clean up my comments
> below.

> I also noticed a few other bits and knobs that would be good to clean in
> the final patchset. Let me know if you'd like a hand with any of the
> suggestions!

Suggestions are always good. The more eyes on this now, the fewer complaints 
we'll have when end-users get it. 

> 1) I see that this is intended to only be pcbnew. I was thrown by the
> changes to GetSelectMenuText() in the other applications but if I'm
> reading it correctly, that is groundwork for future patches, correct?

I'd originally intended to limit the changes to pcbnew. Then it was suggested 
that the Footprint editor really should get the same treatment, followed by 
people such as yourself saying "what about eeschema?" And some things are 
common across different apps in ways I did not expect; some of the pcbnew DRC 
code is used by the eeschema ERC. 

GetMsgPanelInfo() and GetSelectMenuText() are declared in EDA_ITEM. To ensure 
consistency across all uses I changed the declarations in EDA_ITEM, which 
required the change to ripple throughout all applications. 

I've already started promising follow-up patches to the Footprint editor. 
GerbView needs them too. If we can agree on look-and-feel, I suspect eeschema 
and the symbol editor will follow. I do NOT want to hold up these patches 
waiting on other apps or we'll never break the continuous rebase cycle. 

> 2) The headers in your new files in 0004 seem to keep the copyrights
> from the original files. The copyright on the file itself should not
> extend backwards from the creation date, so any files you create should
> just be (C) 2019.

> 3) Patch 0011 has tabs instead of spaces in a couple places.

Oh frell. If Wayne doesn't chime in saying he's about to merge this patch set 
I'll probably squash these fixes and issue yet another full patch set. 

> 4) In UNIT_BINDER (not UNIT_HELPER -- my mistake below), you pass the
> transform as the last parameter but the data is stored in
> PCB_BASE_FRAME, which is also passed as the first parameter in pcbnew.
> I think we should get this in a similar manner to m_eval in the
> UNIT_BINDER constructor. This would require moving the base definition
> of the origin transform class to EDA_DRAW_FRAME from PCB_BASE_FRAME.
> But I don't think that there's anything pcb-specific about offset/sign
> transform, so moving it to the shared class should not be problematic.

I think I addressed in my previous email why each instantiation of UNIT_BINDER 
needs to be given a parameter identifying the required transform. 

Originally, the origin transform classes had specific knowledge of how to fetch 
the user-selected origin. I expected this to be different for eeschema than for 
pcbnew, as the eeschema implementation might need to calculate the coordinates 
of the user-selected origin corner, while the pcbnew implementation chooses 
from page, aux, or grid origins. Much later I factored out this code and put it 
in PCB_BASE_FRAME. With that done what remains as pcbnew-specific in 
PCB_ORIGIN_TRANSFORMS is the axis inversion selections. 

It's still handy in certain circumstances to have null origin transform 
objects. Specifically, there are places in the code where diagnostic data is 
reported in millimeters, not subject to user-selected units conversion. I've 
intentionally chosen to use null origin transforms in these places so the 
diagnostic data isn't subject to user-selected origin transforms either. 

-Reece 
___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Seth Hillbrand]

On 2019-05-26 11:01, Reece Pollack wrote:

Trust me, I'm not excited about passing the ORIGIN_TRANSFORMS
reference as a parameter. The patch files that do nothing except add
this parameter to the GetMsgPanelInfo() and GetSelectMenuText()
methods are the two biggest in the set, at 1673 and 1488 lines
respectively. The patch files that contain code to actually use this
parameter are far smaller, at 96 and 214 lines, and I kept these
patches separated intentionally.


One option would be to make an overloaded function for GetMsgPanelInfo() 
that takes the origin_transforms parameter and leave the original in 
place.  Then then signatures that need transformation can use it while 
those that don't won't be affected.


This will also reduce the impact of your patch set and make rebasing 
easier.


-S

___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Jeff Young]

The PageLayout Editor is a different beast entirely.  Items have either one 
point (text, image) or two (line, rectangle).  Each point can be anchored to 
one of the four margin corners (or the page top-left).  Each point is 
interpreted as a vector *toward* the centre of the page.

So to draw a box around the printable area, you’d create a rectangle with 
Top-Left of (0,0) anchored to margin-top-left, and a Bottom-Right of (0,0) 
anchored to the margin-bottom-right.

Similarly, to create the title box, you’d create a rectangle with Top-Left of 
(200, 80) anchored to margin-bottom-rigth, and a Bottom-Right of (0,0) anchored 
to margin-bottom-right.

In addition, each item can have a repeat count and step.  So if you step the 
printable area rectangle by (10, 10), the Top-Left corner will move down and to 
the right, while the Bottom-Right will move up and to the left.  This is how we 
get the double-border on the default layout.

Cheers,
Jeff.

> On 26 May 2019, at 16:01, Reece Pollack  wrote:
> 
> Hi Reece-
> 
> I've had a chance to test this a bit.  It works really nicely.  Thanks 
> for the good idea here.
> 
> Thanks!  Just a classic case of "This annoys me so much that I'm going to fix 
> it".
> I only noticed one spot where it wasn't transformed so far: the 
> Measurement tool.  When used, it displays a sign with the distance, 
> which doesn't match the increasing/decreasing convention.
> 
> Thanks. I'm not surprised that I've missed a couple of things. I'll put it on 
> my list of things to fix.
> 
> I'm an electronics hobbyist and I've only done a limited amount with KiCad. 
> I've learned a lot about KiCad features just trying to chase down how to 
> activate code I've changed and I'm still finding stuff I didn't know about.
> 
> The second part is mostly a question.  Where do I set this in Eeschema 
> and page layout?  The setting is in the panel under pcbnew, so I would 
> assume it is a per-application process.  However, there is no other 
> application that has that panel for setting.
> 
> Right now the answer is "you don't."
> 
> I wanted to fix pcbnew because I needed to place components and features in 
> specific locations and having the coordinate origin at the corner of an 
> arbitrary paper drawing made this difficult. My initial patch for v4 changed 
> only the cursor position display in the status bar to be relative to the Grid 
> origin, and later the Aux origin when that became a problem. This was 
> hard-coded and didn't flip the sign of the Y axis. The patches I've submitted 
> are the logical outgrowth of that.
> 
> Schematic entry, however, doesn't really depend on having symbols in specific 
> locations, as long as the representation is clear to the user. Having a 
> page-oriented coordinate display origin was acceptable to me, and the 
> negative Y axis didn't bother me enough to make me change it. From a code 
> perspective, while eeschema has Grid and Aux origins internally I'm not aware 
> of any means in the UI to set them.
> 
> I don't have a strong opinion on whether this should be a KiCad-wide 
> preference or not.  I can't imagine someone wanting to set it one way in 
> Eeschema and another in pcbnew.  If that was your intention, the panel 
> should be at the top level rather than under pcbnew.  If it wasn't, can 
> you give some more insight into why it would be good to split between 
> the applications?
> 
> I have pondered what extending origin transforms to eeschema would look like. 
> I'm not sure I see the value in allowing the setting of arbitrary origins on 
> a schematic. One thought I had was to give the user the option of setting the 
> display origin to one of the four page corners. Selecting upper-left would 
> give the current behavior. Selecting either lower corner would invert the Y 
> axis, and selecting either right corner would invert the X axis. While this 
> could be easily implemented using the framework my current patch set 
> provides, it's quite different from the options needed in pcbnew.
> 
> Just this week I discovered the Page Layout editor has a selection box for 
> page corners. I haven't had time to look at what this does yet.
> 
> Lastly, and this is a bit fundamental, I have reservations about passing 
> this parameter around when it is not needed.  This is more of a C-style 
> convention.  Where functions inherit the frame with the preference, that 
> should be used by a Get() method rather than passing down in a parameter 
> chain.
> 
> Trust me, I'm not excited about passing the ORIGIN_TRANSFORMS reference as a 
> parameter. The patch files that do nothing except add this parameter to the 
> GetMsgPanelInfo() and GetSelectMenuText()  methods are the two biggest in the 
> set, at 1673 and 1488 lines respectively. The patch files that contain code 
> to actually use this parameter are far smaller, at 96 and 214 lines, and I 
> kept these patches separated intentionally. I was hoping to maintain the same 
> separation for DRC-related 

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Reece Pollack]

> Hi Reece-

> I've had a chance to test this a bit. It works really nicely. Thanks
> for the good idea here.

Thanks! Just a classic case of "This annoys me so much that I'm going to fix 
it". 

> I only noticed one spot where it wasn't transformed so far: the
> Measurement tool. When used, it displays a sign with the distance,
> which doesn't match the increasing/decreasing convention.

Thanks. I'm not surprised that I've missed a couple of things. I'll put it on 
my list of things to fix. 

I'm an electronics hobbyist and I've only done a limited amount with KiCad. 
I've learned a lot about KiCad features just trying to chase down how to 
activate code I've changed and I'm still finding stuff I didn't know about. 

> The second part is mostly a question. Where do I set this in Eeschema
> and page layout? The setting is in the panel under pcbnew, so I would
> assume it is a per-application process. However, there is no other
> application that has that panel for setting.

Right now the answer is "you don't." 

I wanted to fix pcbnew because I needed to place components and features in 
specific locations and having the coordinate origin at the corner of an 
arbitrary paper drawing made this difficult. My initial patch for v4 changed 
only the cursor position display in the status bar to be relative to the Grid 
origin, and later the Aux origin when that became a problem. This was 
hard-coded and didn't flip the sign of the Y axis. The patches I've submitted 
are the logical outgrowth of that. 

Schematic entry, however, doesn't really depend on having symbols in specific 
locations, as long as the representation is clear to the user. Having a 
page-oriented coordinate display origin was acceptable to me, and the negative 
Y axis didn't bother me enough to make me change it. From a code perspective, 
while eeschema has Grid and Aux origins internally I'm not aware of any means 
in the UI to set them. 

> I don't have a strong opinion on whether this should be a KiCad-wide
> preference or not. I can't imagine someone wanting to set it one way in
> Eeschema and another in pcbnew. If that was your intention, the panel
> should be at the top level rather than under pcbnew. If it wasn't, can
> you give some more insight into why it would be good to split between
> the applications?

I have pondered what extending origin transforms to eeschema would look like. 
I'm not sure I see the value in allowing the setting of arbitrary origins on a 
schematic. One thought I had was to give the user the option of setting the 
display origin to one of the four page corners. Selecting upper-left would give 
the current behavior. Selecting either lower corner would invert the Y axis, 
and selecting either right corner would invert the X axis. While this could be 
easily implemented using the framework my current patch set provides, it's 
quite different from the options needed in pcbnew. 

Just this week I discovered the Page Layout editor has a selection box for page 
corners. I haven't had time to look at what this does yet. 

> Lastly, and this is a bit fundamental, I have reservations about passing
> this parameter around when it is not needed. This is more of a C-style
> convention. Where functions inherit the frame with the preference, that
> should be used by a Get() method rather than passing down in a parameter
> chain.

Trust me, I'm not excited about passing the ORIGIN_TRANSFORMS reference as a 
parameter. The patch files that do nothing except add this parameter to the 
GetMsgPanelInfo() and GetSelectMenuText() methods are the two biggest in the 
set, at 1673 and 1488 lines respectively. The patch files that contain code to 
actually use this parameter are far smaller, at 96 and 214 lines, and I kept 
these patches separated intentionally. I was hoping to maintain the same 
separation for DRC-related changes but that just got too messy. 

The problem is that the classes that perform the display formatting are part of 
the hierarchy that implement the board or schematic, which is separate from the 
class hierarchy that implements the board and schematic editors. The board 
editor has a pointer to the board but there's no link the other way, and the 
ORIGIN_TRANSFORMS classes are instantiated in the editor. 

There used to be a global variable called "g_UserUnit" that indicated the 
user's preferred display units, but about this time last year Jeff Young 
replaced it with a parameter passed into the formatting methods. After spending 
several evenings trying to figure out a better way I decided he might know 
something I didn't about the structure of KiCad and decided to follow suit. If 
you want to suggest a better way to do it, I'm all ears. 

> In some cases (UNIT_HELPER), this should either be incorporated into
> UNIT_HELPER or written as a class that inherits UNIT_HELPER. The class
> then gets the current setting (as Unit helper does with the units) and
> applies it in one place only.

The specific problem 

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Seth Hillbrand]

Hi Reece-

Apologies for the double-tap here.  I see I miss-read your intention in 
a couple of places!  Sorry about that.  I'll try to clean up my comments 
below.


I also noticed a few other bits and knobs that would be good to clean in 
the final patchset.  Let me know if you'd like a hand with any of the 
suggestions!


1) I see that this is intended to only be pcbnew.  I was thrown by the 
changes to GetSelectMenuText() in the other applications but if I'm 
reading it correctly, that is groundwork for future patches, correct?


2) The headers in your new files in 0004 seem to keep the copyrights 
from the original files.  The copyright on the file itself should not 
extend backwards from the creation date, so any files you create should 
just be (C) 2019.


3) Patch 0011 has tabs instead of spaces in a couple places.

4) In UNIT_BINDER (not UNIT_HELPER -- my mistake below), you pass the 
transform as the last parameter but the data is stored in 
PCB_BASE_FRAME, which is also passed as the first parameter in pcbnew.  
I think we should get this in a similar manner to m_eval in the 
UNIT_BINDER constructor.  This would require moving the base definition 
of the origin transform class to EDA_DRAW_FRAME from PCB_BASE_FRAME.  
But I don't think that there's anything pcb-specific about offset/sign 
transform, so moving it to the shared class should not be problematic.


Best-
Seth

On 2019-05-25 22:57, Seth Hillbrand wrote:

Hi Reece-

I've had a chance to test this a bit.  It works really nicely.  Thanks
for the good idea here.

I only noticed one spot where it wasn't transformed so far: the
Measurement tool.  When used, it displays a sign with the distance,
which doesn't match the increasing/decreasing convention.

The second part is mostly a question.  Where do I set this in Eeschema
and page layout?  The setting is in the panel under pcbnew, so I would
assume it is a per-application process.  However, there is no other
application that has that panel for setting.

I don't have a strong opinion on whether this should be a KiCad-wide
preference or not.  I can't imagine someone wanting to set it one way
in Eeschema and another in pcbnew.  If that was your intention, the
panel should be at the top level rather than under pcbnew.  If it
wasn't, can you give some more insight into why it would be good to
split between the applications?

Lastly, and this is a bit fundamental, I have reservations about
passing this parameter around when it is not needed.  This is more of
a C-style convention.  Where functions inherit the frame with the
preference, that should be used by a Get() method rather than passing
down in a parameter chain.

In some cases (UNIT_HELPER), this should either be incorporated into
UNIT_HELPER or written as a class that inherits UNIT_HELPER.  The
class then gets the current setting (as Unit helper does with the
units) and applies it in one place only.

The problem with the current solution is that it becomes very
difficult to maintain as the parameter count increases.

Overall, this is an excellent piece of work.  I look forward to using
it and flipping my perspective around. :)

Best-
Seth



On 2019-05-25 09:08, Reece R. Pollack wrote:

The Zip file attachment contains the complete set of patches
implementing Display Origin Transforms, now squashed and rebased for
your merging pleasure!

They should apply cleanly atop this commit from JP Charras:


b8e2054 Activate context menu in LIB_VIEW_FRAME canvas.

 Folks, please resist the urge to commit another 110 patches before
Wayne has a chance to merge these onto master. :-)

-Reece
___
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


___
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


___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms rebase

[Seth Hillbrand]

Hi Reece-

I've had a chance to test this a bit.  It works really nicely.  Thanks 
for the good idea here.


I only noticed one spot where it wasn't transformed so far: the 
Measurement tool.  When used, it displays a sign with the distance, 
which doesn't match the increasing/decreasing convention.


The second part is mostly a question.  Where do I set this in Eeschema 
and page layout?  The setting is in the panel under pcbnew, so I would 
assume it is a per-application process.  However, there is no other 
application that has that panel for setting.


I don't have a strong opinion on whether this should be a KiCad-wide 
preference or not.  I can't imagine someone wanting to set it one way in 
Eeschema and another in pcbnew.  If that was your intention, the panel 
should be at the top level rather than under pcbnew.  If it wasn't, can 
you give some more insight into why it would be good to split between 
the applications?


Lastly, and this is a bit fundamental, I have reservations about passing 
this parameter around when it is not needed.  This is more of a C-style 
convention.  Where functions inherit the frame with the preference, that 
should be used by a Get() method rather than passing down in a parameter 
chain.


In some cases (UNIT_HELPER), this should either be incorporated into 
UNIT_HELPER or written as a class that inherits UNIT_HELPER.  The class 
then gets the current setting (as Unit helper does with the units) and 
applies it in one place only.


The problem with the current solution is that it becomes very difficult 
to maintain as the parameter count increases.


Overall, this is an excellent piece of work.  I look forward to using it 
and flipping my perspective around. :)


Best-
Seth



On 2019-05-25 09:08, Reece R. Pollack wrote:

The Zip file attachment contains the complete set of patches
implementing Display Origin Transforms, now squashed and rebased for
your merging pleasure!

They should apply cleanly atop this commit from JP Charras:


b8e2054 Activate context menu in LIB_VIEW_FRAME canvas.

 Folks, please resist the urge to commit another 110 patches before
Wayne has a chance to merge these onto master. :-)

-Reece
___
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


___
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

[Kicad-developers] Patch set: Display Origin Transforms rebase

[Reece R. Pollack]

The Zip file attachment contains the complete set of patches 
implementing Display Origin Transforms, now squashed and rebased for 
your merging pleasure!


They should apply cleanly atop this commit from JP Charras:

   b8e2054 Activate context menu in LIB_VIEW_FRAME canvas.

Folks, please resist the urge to commit another 110 patches before Wayne 
has a chance to merge these onto master. :-)


-Reece
<>
___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms updates

[Wayne Stambaugh]

Reece,

I hate to ask you to rebase again but there has been so much activity
recently that several of you original patches no longer apply cleanly.
Please rebase and resubmit the original patch set when you get a chance.

Lead Devs,

Let's try to be kind to Reece so I can get this merged.  As soon as he
resubmits his rebased patch set, please hold off on pushing any new
changes until I can get it merged.  Otherwise we will continue this
never ending loop.

Thanks,

Wayne

On 5/23/2019 9:49 PM, Reece R. Pollack wrote:
> I've attached a Zip file containing additional patches to be applied on
> top of the previous set.
> 
> The first three patch files address Wayne's code review comments. If
> anyone else had comments I didn't see them.
> 
> The fourth patch file provides support for Bezier control points. A big
> "thank you" goes to Seth Hillbrand for providing the example PCB file,
> which was very helpful.
> 
> The remaining two patch files provide support for the "Move Exactly",
> "Move Item", and "Create Array" dialogs which the first patch set didn't
> address.
> 
> I think this covers all of the places where Pcbnew displays coordinates
> and offsets to the user. If anyone sees something that is either not
> adjusted for the user-selected origin, or has the wrong sign, please let
> me know!
> 
> -Reece
> 
> 
> ___
> 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
> 

___
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

[Kicad-developers] Patch set: Display Origin Transforms updates

[Reece R. Pollack]

I've attached a Zip file containing additional patches to be applied on 
top of the previous set.


The first three patch files address Wayne's code review comments. If 
anyone else had comments I didn't see them.


The fourth patch file provides support for Bezier control points. A big 
"thank you" goes to Seth Hillbrand for providing the example PCB file, 
which was very helpful.


The remaining two patch files provide support for the "Move Exactly", 
"Move Item", and "Create Array" dialogs which the first patch set didn't 
address.


I think this covers all of the places where Pcbnew displays coordinates 
and offsets to the user. If anyone sees something that is either not 
adjusted for the user-selected origin, or has the wrong sign, please let 
me know!


-Reece

<>
___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms

[Reece Pollack]

Wayne, 

Thanks for the feedback. 

Re the header guards: It's common in my experience to define include/exclude 
type symbols with a non-zero numeric value. This avoids problems when someone 
writes "#if SYMBOL" rather than the intended "#ifdef SYMBOL", as a preprocessor 
symbol with no value evaluates to zero in this context. The majority, though 
certainly not all, of the include files in /usr/include follow this convention 
for this very reason. 

I'll get you a patch to address the "const" parameters and your other comments 
as soon as I get a chance. A patch for "Move Exactly" and Bezier will follow in 
a few days. 

-Reece 


From: "Wayne Stambaugh"  
To: "KiCad Developers"  
Sent: Tuesday, May 21, 2019 10:21:10 AM 
Subject: Re: [Kicad-developers] Patch set: Display Origin Transforms 

Hey Reece, 

I tested your patch set and everything seems to work as advertised. I 
have a few minor comments: 

The ORIGIN_TRANSFORMS references should be passed as const in all places 
where they are used internally by another object that doesn't modify them. 

It's not necessary to add '1' to the header compile guards. AFAIK, none 
of the other header files do this. 

I'm fine if these fixes are implemented in a separate patch. 

I'm fine with merging this patch set as long as there are no objections. 

I answered some of your questions in-line below. 

On 5/19/19 10:13 PM, Reece R. Pollack wrote: 
> I've attached a Zip file containing 11 patches. These implement the 
> Origin Transforms feature I've been talking about since KiCon. They 
> should apply cleanly to the master branch at this commit (currently HEAD): 
> 
> 9d56102 Prevent unannotated components from driving connectivity 
> 
> In summary, this adds Pcbnew user preferences that allow the user to 
> select the origin from which absolute coordinates are displayed and 
> entered. The supported origins are the Page Origin, the Auxiliary 
> Origin, and the Grid Origin. If the user preference has not been set the 
> default is Page Origin, which looks just like what we have now. 
> 
> Additionally, two other Pcbnew user preferences are added to allow the 
> user to select which way the X and Y axes increase: Left or Right for 
> the X axis, and Up or Down for the Y axis. If the user preference has 
> not been set the default is X Right and Y Down, which again looks just 
> like what we have now. 
> 
> I added a new panel to the Pcbnew "Preferences" dialog called "Origins & 
> Axes" to allow the user to change these options. I did not add any 
> toolbar icons as I expect these will be "set and forget" options for 
> most users. 
> 
> These patches do not alter the content of the board file, nor do they 
> change the internal representation of coordinates. The user can change 
> preferences without causing revision-managed data churn. The only affect 
> is how the user sees and enters coordinate values. 
> 
> My intent has been to implement these transforms only in Pcbnew, but the 
> changes to common data structures necessarily affect all KiCad 
> applications. Thus support for display origin and axis shifts is latent 
> in the Footprint Editor, GerbView, Eeschema, and the Symbol Editor, and 
> can be implemented with minimal effort. However, at this point there 
> should be no user-visible changes in any of these applications. 
> 
> Some notes: 
> 
> 1. The new file "origin_transform.cpp" is currently in common/widgets/ 
> because that's where unit_binder.cpp was located. It might ought to 
> be in common/ instead. 

Should be in common. It's not really a widget object. 

> 2. I believe I've addressed all user-visible Pcbnew displays and dialog 
> boxes other than the Move Exactly dialog. If I missed something 
> else, let me know. 
> 3. I haven't decided how the "Move Exactly" dialog should work yet; I 
> think it needs axis orientation support but not origin translation. 
> I'd be happy to get feedback before I code a patch for this. 

It probably should be implemented in the move exactly dialog for 
absolution positions. 

> 4. I did not touch the Bezier coordinates because it appears this is 
> not fully implemented in Pcbnew and I couldn't figure out how I 
> would test such changes. 

I thought we did go live with this recently so Bezier coordinates will 
need to be supported unless this was only in the footprint editor. 

> 5. I'm willing to make a pass through the code to unify the name of the 
> Auxiliary Origin once there is a consensus on what to call it. 

By auxiliary origin, I'm assuming you mean the place and drill origin in 
pcbnew. If so, the latter is more descriptive. Auxiliary origin is not 
very descriptive. 

> 6. Patching the file containing the list of developers to add 

Re: [Kicad-developers] Patch set: Display Origin Transforms

[Seth Hillbrand]

Am 2019-05-21 10:21, schrieb Wayne Stambaugh:

 4. I did not touch the Bezier coordinates because it appears this is
not fully implemented in Pcbnew and I couldn't figure out how I
would test such changes.


I thought we did go live with this recently so Bezier coordinates will
need to be supported unless this was only in the footprint editor.


Beziers were activated with the new graphics import routines.  You can 
import a bezier from a dxf.  I'm working on a transformations patch 
between lines/arcs/beziers that will expose this to more easier user 
access.


Until then, let me know if you'd like a .kicad_pcb file with beziers 
that you can move around to test things.


-Seth

___
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

Re: [Kicad-developers] Patch set: Display Origin Transforms

[Wayne Stambaugh]

Hey Reece,

I tested your patch set and everything seems to work as advertised.  I
have a few minor comments:

The ORIGIN_TRANSFORMS references should be passed as const in all places
where they are used internally by another object that doesn't modify them.

It's not necessary to add '1' to the header compile guards.  AFAIK, none
of the other header files do this.

I'm fine if these fixes are implemented in a separate patch.

I'm fine with merging this patch set as long as there are no objections.

I answered some of your questions in-line below.

On 5/19/19 10:13 PM, Reece R. Pollack wrote:
> I've attached a Zip file containing 11 patches. These implement the
> Origin Transforms feature I've been talking about since KiCon. They
> should apply cleanly to the master branch at this commit (currently HEAD):
> 
> 9d56102 Prevent unannotated components from driving connectivity
> 
> In summary, this adds Pcbnew user preferences that allow the user to
> select the origin from which absolute coordinates are displayed and
> entered. The supported origins are the Page Origin, the Auxiliary
> Origin, and the Grid Origin. If the user preference has not been set the
> default is Page Origin, which looks just like what we have now.
> 
> Additionally, two other Pcbnew user preferences are added to allow the
> user to select which way the X and Y axes increase: Left or Right for
> the X axis, and Up or Down for the Y axis. If the user preference has
> not been set the default is X Right and Y Down, which again looks just
> like what we have now.
> 
> I added a new panel to the Pcbnew "Preferences" dialog called "Origins &
> Axes" to allow the user to change these options. I did not add any
> toolbar icons as I expect these will be "set and forget" options for
> most users.
> 
> These patches do not alter the content of the board file, nor do they
> change the internal representation of coordinates. The user can change
> preferences without causing revision-managed data churn. The only affect
> is how the user sees and enters coordinate values.
> 
> My intent has been to implement these transforms only in Pcbnew, but the
> changes to common data structures necessarily affect all KiCad
> applications. Thus support for display origin and axis shifts is latent
> in the Footprint Editor, GerbView, Eeschema, and the Symbol Editor, and
> can be implemented with minimal effort. However, at this point there
> should be no user-visible changes in any of these applications.
> 
> Some notes:
> 
>  1. The new file "origin_transform.cpp" is currently in common/widgets/
> because that's where unit_binder.cpp was located. It might ought to
> be in common/ instead.

Should be in common.  It's not really a widget object.

>  2. I believe I've addressed all user-visible Pcbnew displays and dialog
> boxes other than the Move Exactly dialog. If I missed something
> else, let me know.
>  3. I haven't decided how the "Move Exactly" dialog should work yet; I
> think it needs axis orientation support but not origin translation.
> I'd be happy to get feedback before I code a patch for this.

It probably should be implemented in the move exactly dialog for
absolution positions.

>  4. I did not touch the Bezier coordinates because it appears this is
> not fully implemented in Pcbnew and I couldn't figure out how I
> would test such changes.

I thought we did go live with this recently so Bezier coordinates will
need to be supported unless this was only in the footprint editor.

>  5. I'm willing to make a pass through the code to unify the name of the
> Auxiliary Origin once there is a consensus on what to call it.

By auxiliary origin, I'm assuming you mean the place and drill origin in
pcbnew.  If so, the latter is more descriptive.  Auxiliary origin is not
very descriptive.

>  6. Patching the file containing the list of developers to add my name
> felt kinda presumptuous. I'd be happy if these patches constitutes
> cause to do so.

I will do that once your patches are merged.

>  7. Would someone send Jeff Young on holiday for a week or two? I'm
> getting burned out just trying to keep these patches rebased on his
> changes. :-)

Jeff, you want to field this question?

> 
> 
> -Reece
> 
> 
> 
> ___
> 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
> 

___
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

[Kicad-developers] Patch set: Display Origin Transforms

[Reece R. Pollack]

I've attached a Zip file containing 11 patches. These implement the 
Origin Transforms feature I've been talking about since KiCon. They 
should apply cleanly to the master branch at this commit (currently HEAD):


   9d56102 Prevent unannotated components from driving connectivity

In summary, this adds Pcbnew user preferences that allow the user to 
select the origin from which absolute coordinates are displayed and 
entered. The supported origins are the Page Origin, the Auxiliary 
Origin, and the Grid Origin. If the user preference has not been set the 
default is Page Origin, which looks just like what we have now.


Additionally, two other Pcbnew user preferences are added to allow the 
user to select which way the X and Y axes increase: Left or Right for 
the X axis, and Up or Down for the Y axis. If the user preference has 
not been set the default is X Right and Y Down, which again looks just 
like what we have now.


I added a new panel to the Pcbnew "Preferences" dialog called "Origins & 
Axes" to allow the user to change these options. I did not add any 
toolbar icons as I expect these will be "set and forget" options for 
most users.


These patches do not alter the content of the board file, nor do they 
change the internal representation of coordinates. The user can change 
preferences without causing revision-managed data churn. The only affect 
is how the user sees and enters coordinate values.


My intent has been to implement these transforms only in Pcbnew, but the 
changes to common data structures necessarily affect all KiCad 
applications. Thus support for display origin and axis shifts is latent 
in the Footprint Editor, GerbView, Eeschema, and the Symbol Editor, and 
can be implemented with minimal effort. However, at this point there 
should be no user-visible changes in any of these applications.


Some notes:

1. The new file "origin_transform.cpp" is currently in common/widgets/
   because that's where unit_binder.cpp was located. It might ought to
   be in common/ instead.
2. I believe I've addressed all user-visible Pcbnew displays and dialog
   boxes other than the Move Exactly dialog. If I missed something
   else, let me know.
3. I haven't decided how the "Move Exactly" dialog should work yet; I
   think it needs axis orientation support but not origin translation.
   I'd be happy to get feedback before I code a patch for this.
4. I did not touch the Bezier coordinates because it appears this is
   not fully implemented in Pcbnew and I couldn't figure out how I
   would test such changes.
5. I'm willing to make a pass through the code to unify the name of the
   Auxiliary Origin once there is a consensus on what to call it.
6. Patching the file containing the list of developers to add my name
   felt kinda presumptuous. I'd be happy if these patches constitutes
   cause to do so.
7. Would someone send Jeff Young on holiday for a week or two? I'm
   getting burned out just trying to keep these patches rebased on his
   changes. :-)


-Reece


<>
___
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