Re: [Kicad-developers] Display origin transforms for DRC reports?
It sounds like the consensus approach is to add a second parameter to the GetSelectMenuText() function, which will be a pointer or reference to an ORIGIN_TRANSFORMS object. Those implementations of this function that format coordinates will use this to display the coordinates relative to the user-selected origin. I propose that if it is desired to move the user origin and in/mm selection from the user display options to the project be deferred to a separate patch set so this can be discussed independently. Does this sound right to everyone? -Reece On 7/10/20 4:56 PM, Ian McInerney wrote: I view the package of information need as the arguments to the function. If information isn't needed, then it shouldn't be passed in. Creating a new wrapper type that just holds the EDA_UNITS and the ORIGIN_TRANSFORMS object seems like an extra level of abstraction that is not needed. The point of my original question about the IU is that if the transform operates on the IU, then these two items are probably not needed together anywhere else. Adding a struct solely to hold these two datatypes will add code/understanding overhead to the functions and its callsite, specifically: * Having to create the actual struct and populate it's fields before calling the function * In the function the struct has to be unpacked/continually referenced (so the variable names become longer) * It hides the fact that we are passing these two items (one of which is already an opaque type) inside another opaque type when someone just views the prototype for the function To me, it seems like there is not enough here to justify the creation of a new layer of abstraction to pass these two items into this function. If these two datatypes are needed in other places at the same time, then I could see the justification. -Ian ___ 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] Display origin transforms for DRC reports?
Presumably the combination of units+transform is also needed for things like plotting, no? (where the chosen set of units and transform may be different from those used for display) On Fri, Jul 10, 2020 at 4:56 PM Ian McInerney wrote: > > I view the package of information need as the arguments to the function. If > information isn't needed, then it shouldn't be passed in. Creating a new > wrapper type that just holds the EDA_UNITS and the ORIGIN_TRANSFORMS object > seems like an extra level of abstraction that is not needed. The point of my > original question about the IU is that if the transform operates on the IU, > then these two items are probably not needed together anywhere else. Adding a > struct solely to hold these two datatypes will add code/understanding > overhead to the functions and its callsite, specifically: > * Having to create the actual struct and populate it's fields before calling > the function > * In the function the struct has to be unpacked/continually referenced (so > the variable names become longer) > * It hides the fact that we are passing these two items (one of which is > already an opaque type) inside another opaque type when someone just views > the prototype for the function > > To me, it seems like there is not enough here to justify the creation of a > new layer of abstraction to pass these two items into this function. If these > two datatypes are needed in other places at the same time, then I could see > the justification. > > -Ian > > On Fri, Jul 10, 2020 at 9:30 PM Jon Evans wrote: >> >> The way I see it, there is one package of information that we should >> be delivering: how to turn internal units into real text for display. >> That information includes both a units selection and whatever >> transform to apply in case there are coordinates involved. >> >> I see no benefit into splitting that into two arguments. >> >> On Fri, Jul 10, 2020 at 4:27 PM Ian McInerney >> wrote: >> > >> > Doesn't the origin transform operate on the IU and not use the units >> > directly (so the units are only needed by the actual menu text generation >> > function)? If they are separate, then we should pass them as two >> > individual arguments to the GetSelectMenuText function. >> > >> > -Ian >> > >> > On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: >> >> >> >> It would be preferable to make it possible to call GetSelectMenuText >> >> (or really, any facility that needs access to the current units and >> >> coordinate transform) without a frame. >> >> >> >> We have recently been putting a lot of effort into separating the >> >> logic code from the UI / frame code in KiCad, and it would be a shame >> >> to introduce something that ties them together. >> >> >> >> What about bundling together the units selection and the origin >> >> transforms into a single object that can be passed into >> >> GetSelectMenuText instead of the EDA_DRAW_FRAME? >> >> >> >> That would make it easier to write testcases and other standalone >> >> chunks of code that can pass in arbitrary units and transforms without >> >> having to construct a frame. >> >> >> >> -Jon >> >> >> >> On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: >> >> > >> >> > The units selection and the origin transforms are both available through >> >> > the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are >> >> > a combined 118 calls and function declarations), but it appears that >> >> > when the caller has access to an EDA_DRAW_FRAME then the call is some >> >> > variant of: >> >> > >> >> > GetSelectMenuText( m_editFrame->GetUserUnits() ); >> >> > >> >> > Where there isn't access to an EDA_DRAW_FRAME, it's coded as: >> >> > >> >> > GetSelectMenuText( EDA_UNITS::MILLIMETRES ); >> >> > >> >> > It seems to me that if we're going to change the parameters to this >> >> > function, we should pass a pointer to an EDA_DRAW_FRAME as a single >> >> > parameter. If this parameter is nullptr, then we assume >> >> > EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter >> >> > functions in EDA_DRAW_FRAME to get the correct object or data. >> >> > >> >> > I'd need to see more than a few nodding heads before I made either of >> >> > these changes. >> >> > >> >> > -Reece >> >> > >> >> > On 7/10/20 3:00 PM, Jeff Young wrote: >> >> > > 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
I view the package of information need as the arguments to the function. If information isn't needed, then it shouldn't be passed in. Creating a new wrapper type that just holds the EDA_UNITS and the ORIGIN_TRANSFORMS object seems like an extra level of abstraction that is not needed. The point of my original question about the IU is that if the transform operates on the IU, then these two items are probably not needed together anywhere else. Adding a struct solely to hold these two datatypes will add code/understanding overhead to the functions and its callsite, specifically: * Having to create the actual struct and populate it's fields before calling the function * In the function the struct has to be unpacked/continually referenced (so the variable names become longer) * It hides the fact that we are passing these two items (one of which is already an opaque type) inside another opaque type when someone just views the prototype for the function To me, it seems like there is not enough here to justify the creation of a new layer of abstraction to pass these two items into this function. If these two datatypes are needed in other places at the same time, then I could see the justification. -Ian On Fri, Jul 10, 2020 at 9:30 PM Jon Evans wrote: > The way I see it, there is one package of information that we should > be delivering: how to turn internal units into real text for display. > That information includes both a units selection and whatever > transform to apply in case there are coordinates involved. > > I see no benefit into splitting that into two arguments. > > On Fri, Jul 10, 2020 at 4:27 PM Ian McInerney > wrote: > > > > Doesn't the origin transform operate on the IU and not use the units > directly (so the units are only needed by the actual menu text generation > function)? If they are separate, then we should pass them as two individual > arguments to the GetSelectMenuText function. > > > > -Ian > > > > On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: > >> > >> It would be preferable to make it possible to call GetSelectMenuText > >> (or really, any facility that needs access to the current units and > >> coordinate transform) without a frame. > >> > >> We have recently been putting a lot of effort into separating the > >> logic code from the UI / frame code in KiCad, and it would be a shame > >> to introduce something that ties them together. > >> > >> What about bundling together the units selection and the origin > >> transforms into a single object that can be passed into > >> GetSelectMenuText instead of the EDA_DRAW_FRAME? > >> > >> That would make it easier to write testcases and other standalone > >> chunks of code that can pass in arbitrary units and transforms without > >> having to construct a frame. > >> > >> -Jon > >> > >> On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: > >> > > >> > The units selection and the origin transforms are both available > through > >> > the EDA_DRAW_FRAME. I haven't looked at every call in detail (there > are > >> > a combined 118 calls and function declarations), but it appears that > >> > when the caller has access to an EDA_DRAW_FRAME then the call is some > >> > variant of: > >> > > >> > GetSelectMenuText( m_editFrame->GetUserUnits() ); > >> > > >> > Where there isn't access to an EDA_DRAW_FRAME, it's coded as: > >> > > >> > GetSelectMenuText( EDA_UNITS::MILLIMETRES ); > >> > > >> > It seems to me that if we're going to change the parameters to this > >> > function, we should pass a pointer to an EDA_DRAW_FRAME as a single > >> > parameter. If this parameter is nullptr, then we assume > >> > EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the > getter > >> > functions in EDA_DRAW_FRAME to get the correct object or data. > >> > > >> > I'd need to see more than a few nodding heads before I made either of > >> > these changes. > >> > > >> > -Reece > >> > > >> > On 7/10/20 3:00 PM, Jeff Young wrote: > >> > > 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 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 > wrote: > >> > >>> Jon, > >> > >>> > >> > >>> The alternate origins themselves (Place & Drill / Aux origin, and > Grid > >> > >>>
Re: [Kicad-developers] Display origin transforms for DRC reports?
Could we do as Jon suggests and bundle the full affine transformation as a class that we pass to GetSelectMenuText() instead of the current EDA_UNITS? This class could be stored in EDA_DRAW_FRAME for now, replacing the m_userUnits. Once we get around to separating the units from the frame, we can still move this with the same ease as the current system. Seth Hillbrand KiCad Services Corporation https://www.kipro-pcb.com +1 530 302 5483 | +1 212 603 9372 On 2020-07-10 13:42, Jeff Young wrote: Callers of the form: GetSelectMenuText( EDA_UNITS::MILLIMETRES ); are most likely test cases. Dialogs also know what the user units are, so you can get it from there too if you don’t have a frame. But yes, I spent a lot of time divorcing GetSelectMenuText() from global variables and the frame; I’d hate to see it lost. On the topic Jon and Ian were discussing (one parameter or two) I don’t have strong feelings. Cheers, Jeff. On 10 Jul 2020, at 21:30, Jon Evans wrote: The way I see it, there is one package of information that we should be delivering: how to turn internal units into real text for display. That information includes both a units selection and whatever transform to apply in case there are coordinates involved. I see no benefit into splitting that into two arguments. On Fri, Jul 10, 2020 at 4:27 PM Ian McInerney wrote: Doesn't the origin transform operate on the IU and not use the units directly (so the units are only needed by the actual menu text generation function)? If they are separate, then we should pass them as two individual arguments to the GetSelectMenuText function. -Ian On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: It would be preferable to make it possible to call GetSelectMenuText (or really, any facility that needs access to the current units and coordinate transform) without a frame. We have recently been putting a lot of effort into separating the logic code from the UI / frame code in KiCad, and it would be a shame to introduce something that ties them together. What about bundling together the units selection and the origin transforms into a single object that can be passed into GetSelectMenuText instead of the EDA_DRAW_FRAME? That would make it easier to write testcases and other standalone chunks of code that can pass in arbitrary units and transforms without having to construct a frame. -Jon On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: The units selection and the origin transforms are both available through the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are a combined 118 calls and function declarations), but it appears that when the caller has access to an EDA_DRAW_FRAME then the call is some variant of: GetSelectMenuText( m_editFrame->GetUserUnits() ); Where there isn't access to an EDA_DRAW_FRAME, it's coded as: GetSelectMenuText( EDA_UNITS::MILLIMETRES ); It seems to me that if we're going to change the parameters to this function, we should pass a pointer to an EDA_DRAW_FRAME as a single parameter. If this parameter is nullptr, then we assume EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter functions in EDA_DRAW_FRAME to get the correct object or data. I'd need to see more than a few nodding heads before I made either of these changes. -Reece On 7/10/20 3:00 PM, Jeff Young wrote: 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 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 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.
Re: [Kicad-developers] Display origin transforms for DRC reports?
Callers of the form: GetSelectMenuText( EDA_UNITS::MILLIMETRES ); are most likely test cases. Dialogs also know what the user units are, so you can get it from there too if you don’t have a frame. But yes, I spent a lot of time divorcing GetSelectMenuText() from global variables and the frame; I’d hate to see it lost. On the topic Jon and Ian were discussing (one parameter or two) I don’t have strong feelings. Cheers, Jeff. > On 10 Jul 2020, at 21:30, Jon Evans wrote: > > The way I see it, there is one package of information that we should > be delivering: how to turn internal units into real text for display. > That information includes both a units selection and whatever > transform to apply in case there are coordinates involved. > > I see no benefit into splitting that into two arguments. > > On Fri, Jul 10, 2020 at 4:27 PM Ian McInerney > wrote: >> >> Doesn't the origin transform operate on the IU and not use the units >> directly (so the units are only needed by the actual menu text generation >> function)? If they are separate, then we should pass them as two individual >> arguments to the GetSelectMenuText function. >> >> -Ian >> >> On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: >>> >>> It would be preferable to make it possible to call GetSelectMenuText >>> (or really, any facility that needs access to the current units and >>> coordinate transform) without a frame. >>> >>> We have recently been putting a lot of effort into separating the >>> logic code from the UI / frame code in KiCad, and it would be a shame >>> to introduce something that ties them together. >>> >>> What about bundling together the units selection and the origin >>> transforms into a single object that can be passed into >>> GetSelectMenuText instead of the EDA_DRAW_FRAME? >>> >>> That would make it easier to write testcases and other standalone >>> chunks of code that can pass in arbitrary units and transforms without >>> having to construct a frame. >>> >>> -Jon >>> >>> On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: The units selection and the origin transforms are both available through the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are a combined 118 calls and function declarations), but it appears that when the caller has access to an EDA_DRAW_FRAME then the call is some variant of: GetSelectMenuText( m_editFrame->GetUserUnits() ); Where there isn't access to an EDA_DRAW_FRAME, it's coded as: GetSelectMenuText( EDA_UNITS::MILLIMETRES ); It seems to me that if we're going to change the parameters to this function, we should pass a pointer to an EDA_DRAW_FRAME as a single parameter. If this parameter is nullptr, then we assume EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter functions in EDA_DRAW_FRAME to get the correct object or data. I'd need to see more than a few nodding heads before I made either of these changes. -Reece On 7/10/20 3:00 PM, Jeff Young wrote: > 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
The way I see it, there is one package of information that we should be delivering: how to turn internal units into real text for display. That information includes both a units selection and whatever transform to apply in case there are coordinates involved. I see no benefit into splitting that into two arguments. On Fri, Jul 10, 2020 at 4:27 PM Ian McInerney wrote: > > Doesn't the origin transform operate on the IU and not use the units directly > (so the units are only needed by the actual menu text generation function)? > If they are separate, then we should pass them as two individual arguments to > the GetSelectMenuText function. > > -Ian > > On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: >> >> It would be preferable to make it possible to call GetSelectMenuText >> (or really, any facility that needs access to the current units and >> coordinate transform) without a frame. >> >> We have recently been putting a lot of effort into separating the >> logic code from the UI / frame code in KiCad, and it would be a shame >> to introduce something that ties them together. >> >> What about bundling together the units selection and the origin >> transforms into a single object that can be passed into >> GetSelectMenuText instead of the EDA_DRAW_FRAME? >> >> That would make it easier to write testcases and other standalone >> chunks of code that can pass in arbitrary units and transforms without >> having to construct a frame. >> >> -Jon >> >> On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: >> > >> > The units selection and the origin transforms are both available through >> > the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are >> > a combined 118 calls and function declarations), but it appears that >> > when the caller has access to an EDA_DRAW_FRAME then the call is some >> > variant of: >> > >> > GetSelectMenuText( m_editFrame->GetUserUnits() ); >> > >> > Where there isn't access to an EDA_DRAW_FRAME, it's coded as: >> > >> > GetSelectMenuText( EDA_UNITS::MILLIMETRES ); >> > >> > It seems to me that if we're going to change the parameters to this >> > function, we should pass a pointer to an EDA_DRAW_FRAME as a single >> > parameter. If this parameter is nullptr, then we assume >> > EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter >> > functions in EDA_DRAW_FRAME to get the correct object or data. >> > >> > I'd need to see more than a few nodding heads before I made either of >> > these changes. >> > >> > -Reece >> > >> > On 7/10/20 3:00 PM, Jeff Young wrote: >> > > 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
Doesn't the origin transform operate on the IU and not use the units directly (so the units are only needed by the actual menu text generation function)? If they are separate, then we should pass them as two individual arguments to the GetSelectMenuText function. -Ian On Fri, Jul 10, 2020 at 8:40 PM Jon Evans wrote: > It would be preferable to make it possible to call GetSelectMenuText > (or really, any facility that needs access to the current units and > coordinate transform) without a frame. > > We have recently been putting a lot of effort into separating the > logic code from the UI / frame code in KiCad, and it would be a shame > to introduce something that ties them together. > > What about bundling together the units selection and the origin > transforms into a single object that can be passed into > GetSelectMenuText instead of the EDA_DRAW_FRAME? > > That would make it easier to write testcases and other standalone > chunks of code that can pass in arbitrary units and transforms without > having to construct a frame. > > -Jon > > On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: > > > > The units selection and the origin transforms are both available through > > the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are > > a combined 118 calls and function declarations), but it appears that > > when the caller has access to an EDA_DRAW_FRAME then the call is some > > variant of: > > > > GetSelectMenuText( m_editFrame->GetUserUnits() ); > > > > Where there isn't access to an EDA_DRAW_FRAME, it's coded as: > > > > GetSelectMenuText( EDA_UNITS::MILLIMETRES ); > > > > It seems to me that if we're going to change the parameters to this > > function, we should pass a pointer to an EDA_DRAW_FRAME as a single > > parameter. If this parameter is nullptr, then we assume > > EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter > > functions in EDA_DRAW_FRAME to get the correct object or data. > > > > I'd need to see more than a few nodding heads before I made either of > > these changes. > > > > -Reece > > > > On 7/10/20 3:00 PM, Jeff Young wrote: > > > 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 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 > 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
It would be preferable to make it possible to call GetSelectMenuText (or really, any facility that needs access to the current units and coordinate transform) without a frame. We have recently been putting a lot of effort into separating the logic code from the UI / frame code in KiCad, and it would be a shame to introduce something that ties them together. What about bundling together the units selection and the origin transforms into a single object that can be passed into GetSelectMenuText instead of the EDA_DRAW_FRAME? That would make it easier to write testcases and other standalone chunks of code that can pass in arbitrary units and transforms without having to construct a frame. -Jon On Fri, Jul 10, 2020 at 3:33 PM Reece R. Pollack wrote: > > The units selection and the origin transforms are both available through > the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are > a combined 118 calls and function declarations), but it appears that > when the caller has access to an EDA_DRAW_FRAME then the call is some > variant of: > > GetSelectMenuText( m_editFrame->GetUserUnits() ); > > Where there isn't access to an EDA_DRAW_FRAME, it's coded as: > > GetSelectMenuText( EDA_UNITS::MILLIMETRES ); > > It seems to me that if we're going to change the parameters to this > function, we should pass a pointer to an EDA_DRAW_FRAME as a single > parameter. If this parameter is nullptr, then we assume > EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter > functions in EDA_DRAW_FRAME to get the correct object or data. > > I'd need to see more than a few nodding heads before I made either of > these changes. > > -Reece > > On 7/10/20 3:00 PM, Jeff Young wrote: > > 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
The units selection and the origin transforms are both available through the EDA_DRAW_FRAME. I haven't looked at every call in detail (there are a combined 118 calls and function declarations), but it appears that when the caller has access to an EDA_DRAW_FRAME then the call is some variant of: GetSelectMenuText( m_editFrame->GetUserUnits() ); Where there isn't access to an EDA_DRAW_FRAME, it's coded as: GetSelectMenuText( EDA_UNITS::MILLIMETRES ); It seems to me that if we're going to change the parameters to this function, we should pass a pointer to an EDA_DRAW_FRAME as a single parameter. If this parameter is nullptr, then we assume EDA_UNITS::MILLIMETRES and null transforms, otherwise we use the getter functions in EDA_DRAW_FRAME to get the correct object or data. I'd need to see more than a few nodding heads before I made either of these changes. -Reece On 7/10/20 3:00 PM, Jeff Young wrote: 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 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
Even if it's just a view transform, should moving the file to a different computer result in the view transform changing? On Fri, Jul 10, 2020 at 1:59 PM Jeff Young wrote: > > I think that depends on whether the data is stored independently or not. For > instance, our internal values are independent of units, and the display units > are only shown in the EDA_BASE_FRAME. > > If the origin is similar (ie: it’s a view transform), then it should be > passed as a parameter to GetSelectMenuText(). > > However, if the origin affects how to interpret the data in the file, then it > should be in the BOARD/SCHEMATIC like Jon said (which is easy to access from > the BOARD_ITEM or SCH_ITEM). > > Cheers, > Jeff. > > > > On 10 Jul 2020, at 18:25, 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 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 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 :
Re: [Kicad-developers] Display origin transforms for DRC reports?
I think that depends on whether the data is stored independently or not. For instance, our internal values are independent of units, and the display units are only shown in the EDA_BASE_FRAME. If the origin is similar (ie: it’s a view transform), then it should be passed as a parameter to GetSelectMenuText(). However, if the origin affects how to interpret the data in the file, then it should be in the BOARD/SCHEMATIC like Jon said (which is easy to access from the BOARD_ITEM or SCH_ITEM). Cheers, Jeff. > On 10 Jul 2020, at 18:25, 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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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
Re: [Kicad-developers] Display origin transforms for DRC reports?
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 ___ 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] Display origin transforms for DRC reports?
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 ___ 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
