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

Additionally, the code sometimes treats a particular field as if it's just 
arbitrary data, rather than an object. A specific case is in 
DIALOG_GRAPHIC_ITEM_PROPERTIES where the "endX" object usually is an X 
coordinate requiring full transform, but sometimes is a radius that shouldn't 
be transformed. At least I think it shouldn't be transformed. 

-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

Reply via email to