I agree on both points: units and transform should be saved in the project file, and they should be passed to GetSelectMenuText as parameters.
Cheers, Jeff. > On 10 Jul 2020, at 19:35, Jon Evans <j...@craftyjon.com> wrote: > > 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
