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] I've somehow broken my build...

[Jeff Young]

Hi Bernhard,

I had to update CLion because the debugger had started hanging on assert()s.  
The CLion upgrade gave me a “use settings from previous version”, but it 
doesn’t appear to be 100%.  I found a couple of paths that were different so 
I’ve reset them, done a clean, and am starting a full build.  (It also lost my 
inspection settings, but that’s 

I’ve now done a clean and started a full build.  Hopefully this will sort it….

Cheers,
Jeff.


> On 26 May 2019, at 21:41, Bernhard Stegmaier  wrote:
> 
> The “sub-apps” shouldn’t have a Frameworks folder, but use the one from the 
> “parent” Kicad.app (otherwise all the libs would be duplicated).
> 
> Did you maybe issue a “make install” in your build folder?
> After that (I guess due to rewriting the paths, including all the 
> dependencies, and all what happens while creating the bundle) the build 
> folder is pretty borked.
> I tried to continue to use it for usual development, but it had weird errors.
> I usually just delete it after a “make install" and start from scratch.
> 
> 
> Regards,
> Bernhard
> 
>> On 26. May 2019, at 22:07, Jeff Young > > wrote:
>> 
>> I can no longer run pl_editor.
>> 
>> It pukes trying to open dlls in 
>> bin/Kicad.app/Contents/Applications/pl_editor.app/Contents/Frameworks/.
>> 
>> If I copy Frameworks/ from bin/Kicad.app/Contents/Frameworks/ then it works 
>> fine (but nukes it the next time I build).
>> 
>> The other apps don’t have a Frameworks/ in their directory, but they seem to 
>> work fine.
>> 
>> Any ideas?
>> ___
>> 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] I've somehow broken my build...

[Bernhard Stegmaier]

The “sub-apps” shouldn’t have a Frameworks folder, but use the one from the 
“parent” Kicad.app (otherwise all the libs would be duplicated).

Did you maybe issue a “make install” in your build folder?
After that (I guess due to rewriting the paths, including all the dependencies, 
and all what happens while creating the bundle) the build folder is pretty 
borked.
I tried to continue to use it for usual development, but it had weird errors.
I usually just delete it after a “make install" and start from scratch.


Regards,
Bernhard

> On 26. May 2019, at 22:07, Jeff Young  wrote:
> 
> I can no longer run pl_editor.
> 
> It pukes trying to open dlls in 
> bin/Kicad.app/Contents/Applications/pl_editor.app/Contents/Frameworks/.
> 
> If I copy Frameworks/ from bin/Kicad.app/Contents/Frameworks/ then it works 
> fine (but nukes it the next time I build).
> 
> The other apps don’t have a Frameworks/ in their directory, but they seem to 
> work fine.
> 
> Any ideas?
> ___
> 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

[Kicad-developers] I've somehow broken my build...

[Jeff Young]

I can no longer run pl_editor.

It pukes trying to open dlls in 
bin/Kicad.app/Contents/Applications/pl_editor.app/Contents/Frameworks/.

If I copy Frameworks/ from bin/Kicad.app/Contents/Frameworks/ then it works 
fine (but nukes it the next time I build).

The other apps don’t have a Frameworks/ in their directory, but they seem to 
work fine.

Any ideas?___
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 

[Kicad-developers] Why is the schematic view boundary set as small as it is?

[Jon Evans]

Hi all,

This question is particularly for JP since you have written the relevant
code.

Why is SCH_VIEW initialized to such a small size compared to the area that
is accessible if you zoom out?

You can see in my screenshot that the view boundary is initialized to the
pink rectangle, but it is quite easy to zoom out way past that boundary and
move components there.  But because of the boundary being checked in some
places (like zoom to selection), it causes various problems such as the bug
report in [1]

If we really want to set such a small view boundary, we should prevent
moving objects outside of it, and restrict the minimum zoom level to
something more reasonable.  In my opinion, though, we should have some
middle ground -- the current view boundary is too small for some use
cases.   It can be useful to drag the entire sheet contents off-sheet for
some workflows, so I think the view boundary should be at least the size of
a 3x3 grid of worksheets, if not larger than that.

Any thoughts on this?

[image: Screenshot from 2019-05-26 11-30-44.png]

[1] https://bugs.launchpad.net/kicad/+bug/1822442

Thanks,
-Jon
___
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]

> 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