kicad-developers team mailing list archive

[Mário Luzeiro <mrluzeiro@xxxxx>]

Hi Jean-Pierre,

Here is my updates as you suggested.
I make an overall pass with some improvements in the documentation and coding style. Still some missing, let me know if still something really needs attention.

Additional, some improvements more in this patch:
+ fix a bounding box calculation issue.
+ VRML2 parser: add capability to keywords Def Group and USE (should support now VRML2 files converted from FreeCad)

Sorry for this big patch, some additions are very related with other so lots of things was rewritten..
I believe also some more tests are still need and maybe some issues may appear with the use.

Hope you can merge it, thanks!

Regards,
Mario Luzeiro

________________________________________
From: jp charras [jp.charras@xxxxxxxxxx]
Sent: 23 March 2015 20:48
To: Mário Luzeiro; kicad-developers@xxxxxxxxxxxxxxxxxxx
Subject: Re: 3d viewer updates

Le 23/03/2015 12:01, Mário Luzeiro a écrit :
> Hello all,
> here are my new contribution for the 3d-viewer.
>
>
> I feel that I cannot do or think in any major improvements to 3d-viewer in the way it is now.
> I think that changes must be done in the 3d-viewer architecture and integration with kicad in order to possible future improve it.
> So I will propose in a next post some ideas and discussion for future changes.
>
> Jean-Pierre: I notice your update to 5531 (file 3d_draw.cpp), this patch is before that revision so you may find some conflicts in that lines but should be easily solved.
>
> Hope you can merge it!
> Let me know if you would like any code revision / change before merge.
> Thanks!
>
> Regards,
> Mario Luzeiro
>

Thanks. It works fine for me.

However I'll like have some fixes, relative to the coding style policy,
and comments (exactly lack of comments) in headers.
Minor to major issues are:

1 -CBBox.cpp uses sometimes tabs instead spaces.

2 - In classes, public functions and variables do not have their names
starting by an uppercase letter:
For instance, in CIMAGE class:
    void copyFull( ... );
should be:
    void CopyFull( ... );

and
    unsigned char*  m_pixels;
should be:
    unsigned char*  m_Pixels;

3 - By far, the most annoying:
All new headers:
CImage.h
CBBoax.h
vrml_aux.h
and many new methods added to existing headers have no comments.
This means the code maintenance will be not possible soon (at least for
developers who do not have a crystal ball).
At least all public vars and methods need comments.
But also private vars and methods, when they are not obvious.
Of course, not obvious calculations in .cpp files have to be commented.

Could you fix these issues. This is very important, especially comments
which have to be added during the code development, not after it is
finished.
Thanks.

--
Jean-Pierre CHARRAS

Attachment: 20150328_3d-viewer-updates.patch
Description: 20150328_3d-viewer-updates.patch