| Thread Previous • Date Previous • Date Next • Thread Next |
Sorry about the delay in responding. I've been traveling.I don't believe overloading the base instance of GetMsgPanelInfo() and GetSelectMenuText() will achieve the desired goal. Overloading functions works great when the /caller/ wants to choose between variants of a function. In this case, though, the call is being made via a pointer to a EDA_ITEM (in EDA_DRAW_FRAME::SetMsgPanel(), currently at common/legacy_gal/eda_draw_frame.cpp:582). The caller has no knowledge of whether the object requires the third parameter or not. Since EDA_DRAW_FRAME is common to all applications, it's an all or none situation.
Realistically, I expect support for origin transforms will eventually be integrated into all of the applications. Pcbnew first, but users are going to expect consistency from Gerbview. The footprint editor would also benefit greatly, and thus cvpcb too. Okay, maybe the PCB Calculator won't support display origin transforms, and it seems the work sheet editor already has a similar concept.
That leaves Eeschema and the symbol editor. The Aux Origin is defined in Eeschema but there's no way to set it and I'm not sure that's the best way to handle the situation anyway. Although my previous patch set didn't tweak GetMsgPanelInfo() and GetSelectMenuText() in Eeschema code to perform display origin transforms, that could have be done easily since they already receive a reference to an ORIGIN_TRANSFORMS object, which is currently NullOriginTransforms. Then whatever scheme was developed for Eeschema would only require that flavor of ORIGIN_TRANSFORMS to be passed in and these would work. Only the Eeschema flavor of UNIT_BINDER and the appropriate changes to the dialogs would then be needed.
Also consider that much of the DRC marker code is shared between Pcbnew and Eeschema. While Eeschema can simply pass a reference to the NullObjectTransforms object to the common DRC code, this means Eeschema can't be completely isolated from these changes.
The alternative is for someone to come up with a scheme by which the editor hierarchy can provide display formatting functions to the structural hierarchy without passing a parameter to these display-oriented functions. I don't think that's likely, though I'm open to suggestions.
Perhaps the display formatting could be bundled into a class, rather than being classless, global functions as they are now. The editors could then pass a reference to this formatting class in place of the current EDA_UNITS parameter. The display formatting class could handle display units (mm vs in) and origin transformations internally. This could also help ease future formatting enhancements. However, this sort of change would be far broader in scope than my current patch set, and I was hoping for my entrance to the KiCad development community to be a bit less dramatic.
-Reece On 5/26/19 5:57 PM, Wayne Stambaugh wrote:
Seth, I'm fine with this approach. I like your idea of deriving a new UNIT_BINDER object so that it doesn't impact the other frames that do not use ORIGIN_TRANSFORMS. Cheers, Wayne On 5/26/19 4:15 PM, Seth Hillbrand wrote:Hi All- If Reece can separate this so that it doesn't affect eeschema, cvpcb, pagelayout_editor, gerbview or libedit, I'm fine with merging as is and refactoring more later. If we merge this as is, it become much more difficult to fix it later. Since these are all GetMsgPanelInfo() calls, the fix is simply to overload the base instance and then any call that doesn't use ORIGIN_TRANSFORMS should not pass it as a parameter. Once this change is implemented, the rebasing should not be nearly so painful, so the extra holds on commits should not be required. -Seth The fix for this is simply overloading On 2019-05-26 15:54, Jon Evans wrote:Hi Reece, Wayne, Seth, Can you clarify the path forward here? Are we going to merge the second patchset and then do follow-ons to take care of the issues Seth raised, or will there be another full patchset coming? I have a backlog of things to cherry-pick from 5.1 to master that I've been holding on to until this is resolved. Thanks, -Jon On Sun, May 26, 2019 at 2:08 PM Reece R. Pollack <reece@xxxxxxx> wrote:So now it occurs to me that what I should have done was create classes derived from UNIT_BINDER that handle the different types of data (X-abs, Y-abs, X-rel, Y-rel) and instantiated those, rather than adding a parameter to the UNIT_BINDER class. However, that would have also required changing UNIT_BINDER to accept and return _double_ values rather than _int_ to avoid the _int_ -> _double_ -> _int_ -> _double_ conversion sequence formatting for display, and _double_ -> _int_ -> _double_ -> _int_ conversions parsing from display. Maybe I'll do that another time. Too many changes too late for this iteration, I think. On 5/26/19 11:01 AM, Reece Pollack wrote:The specific problem with UNIT_BINDER is that it doesn't know what sort of data it's handling. It could be a value like a track width which shouldn't be altered, a relative coordinate delta which may need a sign flip, or an absolute coordinate which needs both translation and sign flip. Nor does it know whether relative or absolute coordinate is an X or a Y axis. Thus it must either have a parameter identifying the type of data it's handling or a different set of methods to transfer that data in or out based on the type. I chose a constructor parameter, and I chose to pass an object implemented to handle that type of data.
| Thread Previous • Date Previous • Date Next • Thread Next |
