Skip to content

Regression test enhancements: reimport test#163

Open
meantaipan wants to merge 31 commits intogerbv:mainfrom
meantaipan:reimport-test
Open

Regression test enhancements: reimport test#163
meantaipan wants to merge 31 commits intogerbv:mainfrom
meantaipan:reimport-test

Conversation

@meantaipan
Copy link
Copy Markdown
Contributor

This includes a fix for #162 which was discovered when developing this branch.

Regression tests supports some additional command line options:

-i | --reimport        :  Extra export rs274-x, then reimport and check.
-I | --Reimport        :  As above, but fail if images not *identical*

-f | --from-last-err   :  Run from test previously reporting error.
                          If no test failed previously, run from start.

-c | --continue        :  Run from after test previously reporting error.
                          If no test failed previously, exit with message.

-d | --difftool <cmd>  :  Specifies text file diff command.  Will be run as
                            cmd <reference> <output>
                            
-e | --imagetool <cmd> :  Specified image (.png) viewer.  Run as
                            cmd <image>

For example, I start off a test run using

sh run_tests.sh -i -e eog -d meld

then if anything breaks, run again from that point by adding the -f option, or bypass it and run the rest by adding -c.

The -e and -d options make it convenient to examine the results if there are any errors.

Currently, all the tests pass except that reimport does not test the PnP example (because the label text cannot be represented in rs274-x format) and the test-circle-interpolation-1 does not correctly export arcs in quadrant mode.

-I (stringent reimport) does not pass, because there are still pixel differences because of resolution conversion etc. The image comparison process is modified to be a bit more robust for what we want to check:

  • Images are output at constant 300dpi.
  • Diff is thresholded and eroded - if anything in the erosion, then fail.
  • Diff pixels must be against the original image boundary, else fail.

These catch gross errors at about 6-10mil resolution, and also pixel-wide errors that could not be explained by a slight boundary shift or rounding error.

A second rs274-x export file is generated from importing the first, which is compared with the first export output. These must be exactly equal for the test to pass, since import -> export should be idempotent.

…d as RS274-X2.

Note that this is only a single image, so tracks will appear crossed since they
are possibly taken from multiple copper layers.  This will be fixed in next commit.

Also, there is currently no annotation of normal gerbers.  Only the image struct from
the IPC file has the net, refdes, pin etc. attributes.  These will need to be
matched up by coordinate location to other gerber images.
This allows visual splitting of layer info.  e.g.
  gerbv --ipcd356a-layers=0,1,2 foo.ipc foo.ipc foo.ipc --ipcd356a-tracks=y,y,y
will open foo.ipc into three distinct images, for board outline, top and bottom
layers.  Tracks are shown for all three.

Added x2 attributes to the project structure.  This is an easily expandable
way of passing arbitrary options to file readers etc.
Also added some functionality for text (label) display.  Command line
args now allow text color and font size ranges to be entered.
On zooming in, the text will change sizes within bounds so as to
maintain visibility without excessive clutter.

Command line arg --layers added to allow specifying file function
and related attributes for each RS274-X/D file read in.
This is basically a demo of the new search API.  When mouse is
near an object, its standard attributes (or label) will be shown
in the status bar, next to the selected object (if any).
Process tracks as well as pads;
Don't mix track and pad annotations;
Properly handle access=00 in IPC by passing bottom layer number.
Ignore polygon pours unless from aperture macro.
Mouse hover selection of net object will cause other objects in
the same image (layer) to be XOR highlighted if they have matching
netname or refdes.  Obviously, this needs a source of netname and
refdes information, such as IPC-D-356A.

Known issues:

XOR highlighting defeats itself if e.g. a track is generated with
an even number of overlapping strokes.  Proper solution requires
using Cairo instead of deprecated GDK API.
Also added some test cases for IPC-D-356A files.

Changed some golden PNG files to account for pixel differences.
May be because I'm on old OS level (Ubuntu 16.04).
Also added some functionality for text (label) display.  Command line
args now allow text color and font size ranges to be entered.
On zooming in, the text will change sizes within bounds so as to
maintain visibility without excessive clutter.

Command line arg --layers added to allow specifying file function
and related attributes for each RS274-X/D file read in.
This is basically a demo of the new search API.  When mouse is
near an object, its standard attributes (or label) will be shown
in the status bar, next to the selected object (if any).
Process tracks as well as pads;
Don't mix track and pad annotations;
Properly handle access=00 in IPC by passing bottom layer number.
Ignore polygon pours unless from aperture macro.
Mouse hover selection of net object will cause other objects in
the same image (layer) to be XOR highlighted if they have matching
netname or refdes.  Obviously, this needs a source of netname and
refdes information, such as IPC-D-356A.

Known issues:

XOR highlighting defeats itself if e.g. a track is generated with
an even number of overlapping strokes.  Proper solution requires
using Cairo instead of deprecated GDK API.
Also added some test cases for IPC-D-356A files.

Changed some golden PNG files to account for pixel differences.
May be because I'm on old OS level (Ubuntu 16.04).
This adds several steps to test integrity of export/reimport:
1. Do initial image comparison to "golden" reference image.
2. Export as RS274-X
3. Import from step (2), and write as image.
4. Compare image from (1) to image in (3) - should be exact match.
5. Import from step (2), write as RS274-X.
6. Text diff output from step (2) and step (5) - should be exact match
   since after first export, the Gerber file should be in a canonical
   form.

Note that many tests fail the reimport test.  A future commit
will resolve that.
For reimport test, two tests are currently bypassed.  See the FIXME
comment in run_tests.sh.
@eyal0
Copy link
Copy Markdown
Collaborator

eyal0 commented Dec 20, 2022

Can we rebase just this change on to gerbv/main so that it doesn't also include all the other stuff?

If you have trouble with it, let me know. I think that the command that you need to run is like:

git checkout reimport-test
get rebase HEAD^^ --onto main

Something like that? Check that it looks right and then:

git push -f

@meantaipan
Copy link
Copy Markdown
Contributor Author

I think I understand what you are requesting. So HEAD^^ means commit 716d8c2. Tried that but got intimidating nonsense about whitespace. Tabs after spaces, trailing spaces etc. Don't know why it should care, but us mere mortals cannot know the mind of git.

Well that sucks. Instead of that, I'm just going to create a new branch, cherry pick out these changes, and PR for that. You can scrap this PR.

@meantaipan meantaipan closed this Dec 20, 2022
@meantaipan
Copy link
Copy Markdown
Contributor Author

Well I gave it my best shot, but no, too much work since too many other changes interwoven. I would just make a hash of it. So please just make this change dependent on #161 . No hurry. Sorry to let you down, but git always makes me feel stupid so you'll have to live with my limitations...

@meantaipan meantaipan reopened this Dec 20, 2022
@ooxi
Copy link
Copy Markdown
Contributor

ooxi commented Jan 7, 2023

@meantaipan no pressure whatsoever, but it would be great if we could merge some of your changes. I suggest going bit by bit, even if it means more work in total. But it also means your work will go into releases of gerbv faster without any pressure for completion :-)

@meantaipan
Copy link
Copy Markdown
Contributor Author

I'm busy at the moment, but when I get a chance I will take a look at isolating just the fix for #162.

Maybe also the ImageMagick changes to make the regression test image comparisons a bit more robust (not as many false fails).

IIRC, all the other stuff in this fix is fairly dependent on #161, so it doesn't make sense to try to base it directly off the main branch. I don't mind how long it takes, but have you guys got any estimate for how long the IPC changes will take to get accepted? I guess there's a lot of code to inspect etc., but just wondering about the normal pace of these things.

@ooxi
Copy link
Copy Markdown
Contributor

ooxi commented Jan 9, 2023

@meantaipan thanks for the input! I have not yet looked closer at the IPC-D-356A changes, there have seem to be a couple of changes. I'll add a comment in #161

rampageservices pushed a commit to SourceParts/gerbv that referenced this pull request Mar 7, 2026
Without cairo_new_sub_path(), Cairo connects the hole subpath to
the outer aperture shape with an implicit line segment. This causes
rendering artifacts on annular ring pads and any aperture with a
drilled hole.

Add cairo_new_sub_path() in gerbv_draw_aperture_hole() so the hole
is drawn as an independent subpath, matching the expected winding-
rule cutout behavior.

Regenerate 10 affected golden test images.

Extracted from gerbv#163.
rampageservices pushed a commit to SourceParts/gerbv that referenced this pull request Mar 7, 2026
The layer/state duplication logic compared currentNet->layer against
lastLayer, but lastLayer is a *duplicated* pointer that will never
equal a source pointer. This caused every net to create a redundant
duplicate layer and state, wasting O(n) memory.

Track source pointers (srcLayer, srcState) separately and compare
against those to correctly detect when consecutive nets share the
same layer or state, avoiding unnecessary duplication.

Extracted from gerbv#163.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants