kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

On Mon, Nov 04, 2013 at 09:40:55AM +0100, Maciej Sumiński wrote:
> Then how do you recognize holes while drawing? I thought that only
> 'end_contour' needs checking.

No idea, it's only an assumption from the symptoms... see screenshots.
Bottom center, the big catode pad; Bottom right the big hole (it's
a screw hole for a huge heat sink:D)

> It is a matter of approach, I prefer to split model and view to different
> classes. Although - it is possible for drawable items to include routines
> for drawing. An example is selection box, it is drawn by ViewDraw(). The
> role of the PCB_PAINTER is to draw those items that may occur on a PCB.

I know, it's only a philosophic issue :D 

> Correct me if I am wrong, but using vectors may lead to many reallocations
> if you resize it (as you have to assure continuous data space) and deques
> are cheaper for dynamic resizing. Although I see a few spots, where it could

Exactly what it meant. However these are one shot adds and many time
retrieve... and anyway the performance difference would probably swamped
by the opengl/cairo machinery.

> What do you mean by 'crisp' text? In comparison to the default renderer or
> Cairo?

Against the default renderer. Cairo antialiases everything but OTOH is
so slow it's nearly unusable (about 20 seconds for the initial
caching!)... see screenshots, again. It's a nearly unavoidable rounding
issue (in fact there are patents for text hinting systems!); look upper
left the P17/P18 stem width; it's not uniform because rounding happens
in different stages:

- Standard text has scaled vertices and scaled width and it's stroked
  with that width, in pixels - consistent stroke width.

- OpenGL probably scales everything during projection so width is
  subject to rounding. It's possible that result changes with OpenGL
  drivers and/or hints and/or phase of the moon. In fact OpenGL doesn't
  guarantee line widths different from 1, either (look at glLineWidth
  specs).

In short: don't worry about it. Maybe there is an extension somewhere to
fix it but it's not important.

> I am not very familiar with push & shove code, but I guess it should be
> enough. Probably those functions work with copper layers (for which
> LAYER_NUM and GAL_LAYER_NUM should be equal).

That's the idea; however since it call the GetTopLayer there is no
static guarantee it will be a pcb layer instead of an item one, unless
it can be proven that the function is called under that condition. Under
the classic animals OO the problem is like: if I put a tiger in a cage,
nobody ensures me that an elephant will come out... more or less :D
Such a conversion could catch issues like the -1 bitset index.

> I like the idea, but I am not the one to decide whether it should be
> commited or not. As I would like our changes to be merged, I ought to stick
> to the master repository and its rules.

As usual :P if you want it I'm there.

> Anyway, I am really thankful for such a careful code review. Some of bugs
> may become fixed before discovered by users and other remarks gave me a few
> thoughts about the code.

No problem for me... I also do code reviews for a living: when you have
safety related software and electronics certification *is* hairy...  and
some standard (like MISRA-C) are nearly unworkable with (example: you
substantially can't use pointers since you could eventually do bad
arithmetic; good luck with communication buffers).

-- 
Lorenzo Marcantonio
Logos Srl