← Back to team overview

kicad-developers team mailing list archive

Re: Patch set: Display Origin Transforms rebase

 

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@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Follow ups

References