← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix for 3D model offset


On 11/11/2017 01:36 PM, easyw wrote:
> Hi Wayne,
> I agree that probably the simplest and clearest solution it would be
> just to switch the reading depending on which version of pcbnew is the
> file.
> In-fact it could be possible also to preserve the existing offset
> without the need to manually adjust the board.
> If the pcbnew app would convert and write the present offset when saving
> an old pcbnew release to the new format, everything would be preserved.

I want to avoid file version checks as much as possible.  They will just
make a mess of the board file parser and formatter.  This is one of
those rare cases where the only way to fix this is to do a one time
coordinate conversion for a previous versions.

> This would be very useful not particularly for library footprints, which
> have already offset to (0,0,0) but for complex mechanical boards with
> mezzanine or other mechanical parts added to the pcb with i.e. offset as
> for alignment requirements.
> The file format would be maintained with the same parsing, but just the
> reader/writer would fix the difference.

Unless a user used custom models or some of the older library models,
the offset will be 0,0,0 so I suspect the issues will be minimal and
will quickly diminish over time.

> The ironic thing is that the bump in the file format would be created
> for a problem that started from a wrong assumption: that the 3D viewer
> and the exporters would make wrong internal calculation, without just
> testing it.

I'll take this one squarely on chin.  I honestly don't remember why I
didn't change the units from the legacy board format when I wrote the
new board and footprint library file formats.  This reminds me that the
legacy board plugin will have to be modified to convert the model offset
from decimils to mm, for users loading legacy boards and footprints.

> As already suggested by some of the main new 3D devs, a simple solution
> would have had to just document the offset units for 3D models.
> I agree also with you again here :)
>> I'm not sure it's worth the headaches it will cause
> Maurice
> On 11/11/2017 2:49 PM, Wayne Stambaugh wrote:
>> Hold on a second before you prepare a patch.  This would introduce a new
>> semantic to the coordinate entries in the file format which could be
>> open to abuse in the future.  I don't want to open up the possibility of
>> using different units within the file format itself and adding units
>> token will open the door for that possibility.  I will also be confusing
>> to readability because all other units are in mm but do not have the
>> 'mm' token except for 3D model offsets.  I would prefer that we do a one
>> time conversion to mm and bump the file format version rather than add a
>> new token to the file format.  The parser change would be fairly
>> straight forward:
>> if( file_version < VERSION )
>>      convert_units_from_decimils_to_mm();
>> The files will still be readable in previous version of kicad.
>> Obviously users will have to reset the offsets if they are not zero but
>> this typically isn't a big problem.
>> On 11/11/2017 03:57 AM, Oliver Walters wrote:
>>> JP,
>>> Yes I will update with a patch.
>>> On 11 Nov 2017 19:45, "jp charras" <jp.charras@xxxxxxxxxx
>>> <mailto:jp.charras@xxxxxxxxxx>> wrote:
>>>      Le 09/11/2017 à 21:48, Oliver Walters a écrit :
>>>      > JP,
>>>      >
>>>      > I think that 3) is the better option, because it preservers PCB
>>>      file compatibility (as long as the
>>>      > offset is zero). Any time the PCB is saved, the (at (xyz 0 0 0))
>>>      gets written.
>>>      >
>>>      > You are also correct that none of the official library footprints
>>>      have a defined offset. This is why
>>>      > i have never encountered this before.
>>>      So I also think 3 is the better option:
>>>      (at (xyz nn mm ll)) is read as position in inches (old files)
>>>      (offset(xyz nn mm ll)) is read as position in mm (new files)
>>>      And the position is written in file only if not 0 (that is the case
>>>      of our footprint files) to avoid
>>>      breaking the compatibility with older Pcbnew version when not
>>>      mandatory, at least for now.
>>>      Could you prepare a patch?
>>>      Thanks.

Follow ups