Regression test enhancements: reimport test#163
Regression test enhancements: reimport test#163meantaipan wants to merge 31 commits intogerbv:mainfrom
Conversation
…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.
|
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 mainSomething like that? Check that it looks right and then: git push -f |
|
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. |
|
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 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 :-) |
|
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. |
|
@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 |
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.
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.
This includes a fix for #162 which was discovered when developing this branch.
Regression tests supports some additional command line options:
For example, I start off a test run using
sh run_tests.sh -i -e eog -d meldthen if anything breaks, run again from that point by adding the
-foption, or bypass it and run the rest by adding-c.The
-eand-doptions 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: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.