← Back to team overview

kicad-developers team mailing list archive

Re: New zoom code musings.


jean-pierre charras - INPG wrote:
> Wayne Stambaugh a écrit :
>> 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).
> I am afraid drawing sample does not work on Windows (i assume you are 
> talking about samples/drawing/drawing.cpp)
> I tested it with only one changes in code to use a coordinate > 65536 
> when drawing a line.
> in MyCanvas::DrawTestLines( int x, int y, int width, wxDC &dc )
> if it called with x = 0x90000 then when
> dc.SetPen( wxPen( wxT("black"), width, wxDOT) );
> dc.DrawLine( x+20, y+30, 100, y+30 );
> is called (at default scale = 1.0), my PC *freezes* (needs a reset!)
> ( dc.SetPen( wxPen( wxT("black"), width, wxSOLID) );
> dc.DrawLine( x+20, y+20, 100, y+20 );
> called with x = 0x90000 runs Ok).

It seems odd that it only crashes when drawing with the wxDOT pen and
not the wxSOLID pen. Did you try increasing the size of the drawing
area so that it didn't clip? Did you do a debug build? I wonder if
there are any asserts that may provide some useful information. When I
get some time, I'll take a look at it.

> (wxMSW 2.8.9 compiled with gcc 4.2.1, release mode, under XP SP3)
> Be carefull about clipping code in gr_basic.

Thanks for the information. I'll be careful and try not to break anything.

> Unfortunately all coordinates in samples using wxDC use small values 
> and, of course, without patching them there is no problem.
> All my problems where found when using coordinates > 4095 (in arcs) and 
> specially > 16 bits. (at scale 1)
>> 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.
>> 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.
>> Wayne