← Back to team overview

kicad-developers team mailing list archive

Re: New zoom code musings.

 

Wayne Stambaugh wrote:
Warning, rant ahead. If you are only interested in the new zoom stuff,
skip down to the last paragraph. However, if you indulge me, I would
like to impart some wisdom I gathered on my travels through the inner
workings of Kicad and wxWidgets. Let me start by saying that in order
to get the new zoom features implemented I violated at least a half
dozen of my personal coding rules not the least of which are don't
reinvent the wheel and keep it simple. But I was concerned about Dick's
eye sight, so I acquiesced.:} In my travels through the Kicad and
relevant wxWidgets source code, I came to the following conclusion that
all of the drawing coordinate scaling (zooming) and offsetting code in
Kicad is unnecessary and redundant. For all the developers who in the
past have voiced concerns about using floating point for drawing
coordinate calculations, be assured that it is already happening inside
wxDC. Granted, if no changes to the logical or user scalars are made to
the device context, then the coordinates get multiplied by 1.0 * 1.0.
They still get converted to floating point then back to integer. In
fact, all of the scaling code performed as integers in Kicad just adds
clock cycles to what wxDC already does. There is no performance gain by
doing this. Not convinced, check out the "drawing" sample shipped with
the wxWidgets source. It does not scale the drawing object coordinates
before calling the wxDC drawing methods. All it does to change the size
an position of the drawing is manipulate the user scale and logical
offset of the device context. Everything else is taken care of by wxDC
automatically. wxDC even provides functions to translate coordinates
between the logical and display units. All of this capability appears
to have been available in wxDC all the way back to at least 2.2.9 in
December 2001 (that was the oldest wxWidgets help I could find). The
drawing sample works the same on Windows and Linux (64 & 32 bits). Can
someone with OSX confirm that wxWidgets drawing sample works there as
well? If so, I think that all of the code in gr_basic.cpp and all of
the scaling and offset calculation code scattered throughout Kicad
should be remove and pushed down to wxDC where it belongs. This would
greatly simplify drawing in Kicad and having the added bonus of allowing
the use of some of the more advanced wxDC type classes such as
wxGraphicsContext or wxBufferedDC. I am willing to take on the task
(although it will probably take me a few months to complete) after the
next version of Kicad is released. I'll stop ranting now, but I have a
few more in store for a later date.


Good information. Sounds like a good bit of work to change strategy, and tough to straddle a fence while doing it. But if there are no gotchas this seems like a reasonable direction for the package to evolve towards.

Now on to the new zoom code. Due to the coordinate scaling being
scattered throughout Kicad, I did not implement the new zoom code with
#ifdefs all over the place. I started down that path, but it was
quickly becoming an awful mess and a pain to enable and disable. I
tested everything I could think of that is zoom related and it seemed to
work fine for me. There may be some slight single pixel drawing issues
due to rounding since I had to use a scalar to get a zoom of 1.5 (known
as zoom level 15 in the new code). I have not committed the new code to
SVN yet due to talk of a new release in February. If no one has any
objections, I will commit the code in the next few days. I believe that
the risk is fairly low but I will wait. If the consensus is to wait
until after the new release, I'll hold off committing until then.


There are always folks who will not touch the SVN version until we say it is a release candidate. These extra users come on board and essentially become testers of the first RC's. I think we can use that extra testing man-power to find and fix any problems with your changes along with all the hundreds of others. The first RC may not be perfect. That is my opinion, so I would lean towards getting it out there. Given your remarks above, this zoom code would not otherwise exist if we pursue a path of pushing it out of App code and down into wxDC. So if not now, then when?

And if anybody complains, we can always give them a discount. :) In fact, discounts of 100% are feasible.


Dick








Follow ups

References