kicad-developers team mailing list archive

[Reece Pollack <reece@xxxxxxx>]

> 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