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 <re...@his.com> 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 <re...@his.com> 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 <re...@his.com> 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@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