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