kicad-developers team mailing list archive

[Jeff Young <jeff@xxxxxxxxx>]

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 <jon@xxxxxxxxxxxxx> 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 <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
>>