kicad-developers team mailing list archive

[Jon Evans <jon@xxxxxxxxxxxxx>]

OK, I see.

I have mostly stayed out of this conversation before so I will
probably step back, but I would suggest that I think that display
units and origin choice should be a project-level setting, not
system-level.

Probably it makes sense to change GetSelectMenuText so that it has
this context available somehow (whether by passing in an
EDA_DRAW_FRAME or by just passing in the ORIGIN_TRANSFORMS or
whatever)

-Jon

On Fri, Jul 10, 2020 at 2:29 PM Reece R. Pollack <reece@xxxxxxx> wrote:
>
> Jon,
>
> The alternate origins themselves (Place & Drill / Aux origin, and Grid
> origin) in Pcbnew are stored in the BOARD_DESIGN_SETTINGS class and
> saved in the board file. Of course, the Page origin location doesn't
> need to be stored; it's always (0,0).
>
> My initial thought last year was to store the user's choice of display
> origin in the board file, but after some discussion we reached consensus
> that the choice of display origin was more like millimeters/inches and
> thus should be a user option rather than a board property. In the
> proposed implementation, the user's preference for which origin is used
> for display is stored in the PCB_DISPLAY_OPTIONS class and saved in the
> pcbnew.json file. I think a case could be made that this should be saved
> per-project, but no one has made a good argument for that.
>
>
> The primary user of the display origin transforms is the UNIT_BINDER
> class. This class has a pointer to the EDA_DRAW_FRAME object.  Since
> UNIT_BINDER is common, I added a virtual function
> "GetOriginTransforms()" to the EDA_DRAW_FRAME class. This function
> returns a reference to an ORIGIN_TRANSFORMS class. The base
> implementation of the ORIGIN_TRANSFORMS class contains functions that
> return their coordinate arguments unchanged. This way none of the KiCad
> applications see a change in behavior unless they override the
> GetOriginTransforms() function.
>
> In the case of Pcbnew, the EDA_DRAW_FRAME parameter is actually a
> pointer to a PCB_BASE_FRAME object. In this class, the
> GetOriginTransforms() function is overridden and returns a reference to
> a PCB_ORIGIN_TRANSFORMS object. This object's functions know how to
> access the BOARD_DESIGN_SETTINGS and PCB_DISPLAY_OPTIONS objects through
> the PCB_BASE_FRAME object and perform the needed transformations.
>
> This works for everything that can find the EDA_DRAW_FRAME object, like
> UNIT_BINDER. The GetMsgPanelInfo functions take an EDA_DRAW_FRAME
> pointer as a parameter, so this isn't a problem. However, the
> GetSelectMenuText() functions take only an EDA_UNITS parameter. Thus my
> problem.
>
> -Reece
>
> On 7/10/20 1:25 PM, Jon Evans wrote:
> > Shouldn't the origin be stored in the design data (BOARD / SCHEMATIC)
> > rather than the base frame?
> >
> > It seems like all data about objects, including their coordinates in
> > the coordinate space defined by the user, should be available just by
> > parsing a board or schematic file (which should be independent of the
> > EDA_BASE_FRAME)
> >
> > -Jon
> >
> > On Fri, Jul 10, 2020 at 1:18 PM Reece R. Pollack <reece@xxxxxxx> wrote:
> >> Jeff,
> >>
> >> Thanks for the follow-up.
> >>
> >> I'm finding some of the GetSelectMenuText() implementations format
> >> coordinate addresses for display. That means they need to use display
> >> origin transforms so the displayed coordinate matches what the user sees
> >> on the status line and elsewhere. However, there does not appear to be a
> >> path from these functions to the EDA_BASE_FRAME class where these
> >> transforms are currently available.
> >>
> >> Might someone more familiar with the data structures and calling
> >> sequences could suggest how this can best be accomplished?
> >>
> >> Seth, I'd appreciate it if you could bring your knowledge and expertise
> >> to bear.
> >>
> >> -Reece
> >>
> >> On 7/10/20 6:35 AM, Jeff Young wrote:
> >>> No, the DRC re-write won’t affect GetSelectMenuText().  It originated for describing items in the Clarify Selection menu, but it’s now used whenever we want to describe an item to the user.
> >>>
> >>>> On 10 Jul 2020, at 00:51, Reece R. Pollack <reece@xxxxxxx> wrote:
> >>>>
> >>>> On 7/9/20 7:09 PM, Tomasz Wlostowski wrote:
> >>>>> On 10/07/2020 00:58, Reece R. Pollack wrote:
> >>>>>> I'm looking at display origin transformations for DRC reports.
> >>>>>>
> >>>>>> With the 5.1.x branch Pcbnew, the DRC report dialog box message texts
> >>>>>> contained the Cartesian coordinates of each flagged item. It appears
> >>>>>> that the 5.99 branch no longer displays the coordinates of DRC items.
> >>>>>> However, the Cartesian coordinates are still listed in the report file.
> >>>>>> Unlike the display in a dialog box, this report is persistent and could
> >>>>>> be passed to someone using different display origin settings.
> >>>>>>
> >>>>>>    1. Should these coordinates be reported relative to the page origin, or
> >>>>>>       transformed per the user-selected origin and axis directions?
> >>>>>>    2. If the coordinates are transformed, should the report include the
> >>>>>>       user settings?
> >>>>>>
> >>>>>> -Reece
> >>>>>>
> >>>>> Reece,
> >>>>>
> >>>>> I wouldn't introduce any changes to the current DRC code, we're
> >>>>> designing a new DRC engine for KiCad V6 and many things in DRC interface
> >>>>> will likely change.
> >>>>>
> >>>>> IMHO the DRC coordinate transform belongs to the UI, not the DRC itself:
> >>>>> - the DRC engine generates an internal report with coordinates in board
> >>>>> coordinate space
> >>>>> - whatever displays the report to the user (i.e. the DRC dialog)
> >>>>> converts the board coordinates to UI coordinates.
> >>>>>
> >>>>> - my 5 cents
> >>>>> Tom
> >>>>>
> >>>>>
> >>>> Tom,
> >>>>
> >>>> I'm not hard to convince, especially when it means doing less work.
> >>>>
> >>>> This sounds like the RC_ITEM::ShowCoord function will be removed or replaced. It's one of the two troublesome function groups.
> >>>>
> >>>> The other troublesome function group is the GetSelectMenuText function. Last year I knew what they did but I've forgotten in the interim. Will this DRC rewrite replace these?
> >>>>
> >>>> -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
> >>
> >> _______________________________________________
> >> 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
>