Implement full Prototype1b section parsing

[domeitzinger]

This PR implements the full Prototype1b section parsing for DWG files. This section is primarily used to store ACIS and thumbnail data (but also other binary data). It replaces the current ACIS data extraction logic added in #1139 and #1146.

There are still a few unknown values, but the reader should be able to reliably parse the full section. Also it still is relatively low level (e.g. when adding / removing schemas or entries you would have to manually add the appropriate search schema references, data index entries, etc.). This is primarily due to the lack of understanding on what the exact purpose of the search schema is and the relationships between the different records. Therefore it is fully working as a reader, but trying to modify the data would still require some more investigation.

I still left the current ACIS data extraction DXF implementation and only replaced it in the DWG reader logic. As discussed in #1152, that implementation could cause extraction of invalid / corrupted data under specific circumstances and would require a solution similar to this PR.

Tasks done in this PR

• Something awesome.
• An evil bug have been defeated.
• Code cleanup and maintenance has been done.

Related Issues / Pull Requests

• Current Prototype_1b header implementation issues #1152
• Read the ACIS payload of 3DSOLID, REGION and BODY from DXF and DWG #1139
• Read the AcDs payload area offset from the data segment header #1146

[ilCosmico]

Hi @domeitzinger,

I gave this branch a run against our test collection: 423 DWG files, a mix of real drawings from support cases and files written by ACadSharp itself. For every file I extracted the ACIS payloads per handle (count and content hash) with current master and with this branch, then compared the two reports.

• Master extracts 16852 payloads, this branch 13221. Some files drop to zero: one goes from 1448 payloads to 0, another from 336 to 0, a third from 220 to 0, and one more from 312 to 14.
• On one file, 89 payloads keep their count but come back truncated to 154 bytes each, where master returns 3.6 to 6.7 KB of content.
• One file that master reads fine now throws an OverflowException during reading.
• Two files written by ACadSharp's own AcDs writer also lose all their payloads (156 to 0 and 64 to 0), possibly related to sections spanning multiple pages. Both files open cleanly in DWG TrueView, so the section layout itself is accepted by the reference consumer. They are attached below as reproduction files (synthetic test scenes, no customer data).
• I did not find files where this branch recovers payloads that master misses.

To be clear, I think a full section model is the right direction to go, and these look like fixable robustness gaps rather than a problem with the approach itself.

For the failing customer files I can share the parsed section details privately if that helps. Happy to rerun the comparison on iterations of this branch, the harness is ready.

acadsharp_acds_repro.zip

[domeitzinger]

Thanks, here are some notes about your findings:

The amount of ACIS files found can be different due to the section sometimes containing old file data that is not being referenced anymore and no longer being in use, but still stored in the file. Arguably this should not be the case very often as I have barely seen any files with this issue. And additionally I am not just parsing ACIS files with this, but actually all kinds of other files (e.g. thumbnail PNG, etc) as well. The counts you gave somehow indicate this only affects a handful of files where the "payloads" cannot be found. There might be something special about them that I have not encountered in any of my testing files.

Changes in file size could be explainable by the issue I was pointing out that the implementation on the master branch might read data as an ACIS file that does not even belong to it at all (e.g. _data_ record definition bytes or different blob records).

It would be helpful to know where the OverflowException happens and what file caused it, because that could be anything.

As for the 0 byte files: The reason is that the parser reads a file size of 0x62626262 which seems like a special pattern. This might have something to do with the ObjectDataAlignmentOffset shifting that is missing as I had no files to test it with (as mentioned in the comment on line 525).

And files which have been created with the AcDs writer from your open PR are expected not to work (or only work with specific files), as it generates an invalid Prototype1b segment and therefore it cannot be parsed. Otherwise I am not aware of the master branch being able to write the Prototype1b section and therefore it should not be able to find anything.

Also, what exactly do you mean by "extracted the ACIS payloads per handle"?

I cannot really do anything about those issues with no test files, as in all my test files it works flawlessly and I am not experiencing any issues. If it is possible for you to share those test files with me, I would be happy to take a look at this.

[ilCosmico]

Hi @domeitzinger, I took the time to dig into all three findings at the parser level.

About your question on the method first, because it changes how to read my numbers. The harness reads each DWG with DwgReader and hashes ModelerGeometry.AcisData per entity handle, so it measures what a library consumer ends up with. On your branch nothing populates the entities from DataStorage yet, which means a good part of my zero counts, and all of the 154 byte payloads, are just the missing entity wiring, not parser trouble (154 bytes is the small inline block that R2013+ entities carry in the entity stream; master overwrites it with the AcDs payload, your branch leaves it as is). My earlier comment presented those as parser robustness gaps, that one is on me.

So I redid the comparison at section level, calling DataStorage directly, and that isolated the real issue. It is exactly your 0x62626262. The payload area of a _data_ segment is anchored by the segment header field you already parse as ObjectDataAlignmentOffset, in 16 byte units from the segment start (the TODO you left on line 525). The current code aligns to the next 16 byte boundary after the record directory instead. The two coincide except when the directory size is already a multiple of 16, which with 20 byte directory records happens exactly when the record count is a multiple of 4: the writers keep a full 16 byte filler there anyway, filled with 0x62 bytes, hence the pattern. The parser starts 16 bytes early, the first size read returns 0x62626262, everything after that cascades into garbage.

And it is not specific to files written by our PR. A customer file resaved by AutoCAD with 12 records in the data segment fails identically on your branch, first read 0x62626262, all 12 payloads lost, while customer files with 5 and 7 records parse fine. You can reproduce it without any of my files: save 4 solids into a 2013 DWG from AutoCAD. 4 records break, 3 or 5 work. The two repro files from the zip in my previous comment fail for the same reason (156 and 64 records). Once the anchor comes from the header field, all of them should parse.

The OverflowException file is a customer drawing I cannot share, but the parsed section layout is enough to see what happens, so I can just paste it. The current segment index sits at 0x680 (entry 46 of itself), but entry 0 points at 0x80, which holds an older, smaller segidx snapshot from a previous save, 0x80 bytes declared. So your old file data explanation is right. ReadSegments walks every entry and parses that stale segidx too, and readSegmentIndexValue reads FileHeader.SegmentIndexEntryCount entries (53 here) no matter which segment it is actually in. That overruns the 0x80 byte segment, back in ReadSegment the padding size header.SegmentSize - readSize goes negative and ReadBytes throws. For what it is worth, the drawing comes from a vertical application and is almost entirely proxy entities, which probably explains the leftover section content. One more thing worth fixing while you are there: readDsPrototype_1b runs before the objects are read and has no try catch around it, so this one section aborts the whole document load. Master reads the same file fine.

FileHeader: headerSize=128 version=2 revision=10 segIdxOffset=0x680 segIdxEntryCount=53
            schidx=50 datidx=47 search=52 prvsav=49 fileSize=0x16F80 freesp=0
segidx[0]:  offset=0x80    size=0x80     <- stale segidx from a previous save
segidx[4]:  offset=0x500   size=0x180
segidx[46]: offset=0x680   size=0x300    <- current segidx (matches the header)
segidx[47]: offset=0x380   size=0x180
segidx[48]: offset=0x980   size=0x15F00
segidx[49]: offset=0x16880 size=0x100
segidx[50]: offset=0x16980 size=0x200
segidx[51]: offset=0x16B80 size=0x100
segidx[52]: offset=0x16C80 size=0x300
(all other entries are zero)

parsing segment idx=0 at 0x80 (declared size 0x80)
  header: name="segidx" segIdx=0 isBlob=0 segSize=0x80
System.OverflowException: Arithmetic operation resulted in an overflow.
   at ACadSharp.IO.DWG.DwgStreamReaderBase.ReadBytes(Int32 length)
   at ACadSharp.IO.DWG.DwgStreamReaders.DataStorage.ReadSegment()
   at ACadSharp.IO.DWG.DwgStreamReaders.DataStorage.ReadSegments()

If a physical test case for the stale segidx would help, I can build a synthetic file with dummy geometry that reproduces the layout, just say the word. And once the anchor is in I will rerun the corpus, I expect the gap to close almost entirely.

[domeitzinger]

Well yeah, I did not try to do anything with the ModelerGeometry.AcisData as this PR is only for reliably parsing the entire Prototype_1b section. Getting the AcDb3DSolid_ASM_Data schema data and passing it to entities is something that should be done by referencing the DataStorage entries, or linking them in some way. But that is not relevant for the validity of the current implementation.

_data_ section padding bytes

TLDR; I found a mistake in the ODA specifications that caused issues with finding the correct Data start marker position in some scenarios.

I checked your example files and indeed, the reader is on a 16 byte boundary and for some reason it adds an additional 16 bytes of padding. But the multiple of 4 seems to be more of a coincidence. I have multiple files where the count is 8 and no padding is added at all. This has some other unknown reason.

I experimented a bit further with the ObjectDataAlignmentOffset based on the ODA specs and I noticed that the ODA specs actually have a mistake. The calculation of the maxRecordSize for the last record is wrong. ODA states it is (object data alignment offset << 4) + segment size - the record’s stream offset while it should actually be segment size - (object data alignment offset << 4) - the record’s stream offset. This is the reason I did not find a reliable way to use the ObjectDataAlignmentOffset before. Also the ODA docs seem to set the Data start marker at the position after reading the DataHeader entries. This should actually be the stream start position of the _data_ segment + (ObjectDataAlignmentOffset << 4).

Sometimes there are additional data headers (which have valid file contents in the _data_ section) following the ones referred to by the DataIndex though. By using the ObjectDataAlignmentOffset, this would skip all those entries. They seem to be dangling / unreferenced files. I do not quite understand why DWG software would write those unreferenced entries then at all. It is possible to parse them anyways, but as no count is given anywhere and because they are not referenced by anything I think they should actually be omitted.

---

As for your OverflowException, I do not know what you mean by "old file data explanation" and "previous save". It would be helpful to know what segment it is trying to read. The header is always 48 bytes long and that leaves 32 bytes for the actual segment value to read. The only way this could cause an overflow is when the ReadSegmentData reads too much data, or the segidx contains invalid size / offset data.

Also, it does not matter if the readDsPrototype_1b is called before or after readObjects as it is an independent section. An exception would cause either option to crash. None of the other section readers have a try/catch block around them either. And the last thing I would want is an exception to be swallowed or the reader to continue reading and then presenting invalid data. Proxy entities are not stored in the Prototype_1b section and are handled differently. The master branch reads those files fine because it does not try to parse anything at all, it just searches and extracts data.

Any testing material would be helpful, this sounds like an issue very specific to this file.

[ilCosmico]

Glad it converges. The spec correction matches what I hit empirically some time ago: that "data start marker (just after this list of data record headers)" wording in 24.2.2.3 is exactly what broke my reader on AutoCAD files with 9 or more records, where the payload area sits farther out. The only rule that held across every reference file I have is marker = segment start + (ObjectDataAlignmentOffset << 4), which is what #1146 implemented on the read side and what the writer anchors too. Your corrected maxRecordSize also matches the parenthetical the spec itself gives (segment end position minus record start position), so I agree the formula line has the anchor term with the wrong sign.

The count 8 files with no padding fit the same picture: the filler is not mandatory and its size is whatever the anchor implies. The writer keeps the 16 byte gap because the AutoCAD fresh saves it mirrors do that at those counts, but a reader should never infer the marker from padding or alignment, only from the header field.

On the dangling headers, agreed, they look like unreferenced leftovers. Master's extraction picks them up, which also inflates its counts in my comparison, so on the next corpus run I will compare referenced entries only.

On the OverflowException, let me tie it to the dump I posted, since every number there comes from your parser reading that file. The current segidx is the one at 0x680, entry 46 of itself, matching FileHeader.SegmentIndexOffset. Entry 0 points at 0x80 with declared size 0x80, and that segment is itself of type segidx, an older and smaller one. ReadSegments walks it like any other entry, and readSegmentIndexValue loops FileHeader.SegmentIndexEntryCount times (53 here) no matter which segment it is in: 48 bytes of header plus 53 x 12 = 684 bytes to read inside a 128 byte segment. readSize ends up larger than SegmentSize, the padding computation goes negative and ReadBytes overflows. So this is not about swallowing exceptions, and I agree bad data should never be silently presented: if the entry loop is bounded by the segment's own declared size (or segidx type segments met during the walk are skipped, since the authoritative one is already parsed), the exception cannot happen in the first place.

And since testing material helps, I will try to build a synthetic file that reproduces that stale segidx layout with dummy geometry and attach it here.

[ilCosmico]

Rerun done on the updated branch (87a6ba6), same corpus of 423 files, this time comparing at section level and referenced entries only, as promised. Results:

• Every payload the branch extracts now matches the master extraction byte for byte: 3715 handles in common across the corpus, 3715 identical hashes, zero mismatches.
• All the earlier failing cases recovered: the two repro files from the zip (156 and 64 records), the AutoCAD-resaved file with 12 records, the file that came back as 154 byte stubs (its payloads now arrive in full), and the big drops (1448, 336, 220, 312) are all back.
• The branch also surfaces some section entries master never had (thumbnails and other non ACIS records, expected from a full section parser), while the dangling entries are now skipped.
• The payloads master has on top of these are the pre-2013 entity stream ones, out of this section's scope by design.

The only remaining delta on the whole corpus is the OverflowException on the stale segidx file, unchanged as expected. About the synthetic file I promised for it: do you still need it? If you push a fix, the loop above works just as well, I run the real file against your branch and report back. But if a physical test case still helps, for a regression test for example, I will put it together.

Nice work, the anchor fix closed the parsing gap completely.

[domeitzinger]

Thanks for trying.

If I understand correctly, the issue is that the Segment index references a second Segment index section within the header. As far as I can tell from the ODA specs, only a single Segment index is valid per file (the one that is referenced in the file header). The only other way this would make any sense is, that some yet unknown field value might indicate how to handle such cases. But this is probably very specific to that file. It might also just be the case that AutoCAD simply ignores that entry as it is invalid.

Any kind of synthetic or AI generated test files will not be very helpful in finding the issue. Generating a file that still causes the same issue will probably not indicate any solutions to the root problem. As this could probably be due to a combination of yet unknown values, you cannot know which fields are ok to change and which ones are not. It is also worth pointing out that it is important to know how those files were created, what software was used, etc.

I am sorry, but it is a real pain to read through all your verbose AI generated responses. It almost feels like I am talking to a chatbot and I have to filter out all the actually useful information fom the huge wall of text.

[ilCosmico]

Fair point on the length, keeping it short.

No synthetic file then, dropped. In the meantime I asked the customer who owns that drawing for permission to share it: if they agree, I will gladly send it over. And agreed on the segidx: one valid segment index per file, the extra entry is stale data, and skipping it (or bounding the entry loop by the declared segment size) matches what the reference consumers do, since they read that file fine.

The rerun offer stands whenever you update the branch.

[DomCR]

The data storage is misspaced in the project, if this is the class that contains the binary data in the file it should be in the base namespace or in a dedicated one along with the other classes, I don't think it belongs to the DwgStreamReaders.

[DomCR]

I would move the classes related to DataStorage to the namespace ACadSharp.Acis (or a better name 😁)

In separated files please.

[domeitzinger]

Yeah, I kept all in one file to make debugging a bit easier (same reason for the reader inside the DataStorage). I will put the reader from the DataStorage into the DwgPrototype1bReader and move the classes to their separate namespace. I would probably rather use something like ACadSharp.Prototype as the DataStorage is not specific to ACIS files and contains other formats as well e.g. Thumbnail PNG files, etc.

[DomCR]

Is there an advantage to have the reading in the DataStorage class?

The current architecture of the project uses readers like DwgPrototype1bReader to process the information and create the objects, the object should be agnostic to the readers, writers or the format dwg/dxf.

[DomCR]

Please add a try, catch on the reading process to make it safe regarding the result.

[domeitzinger]

Well sure, I can do that, but I left it out as I am thinking of a possible future scenario where the Prototype1b header failed to be read and then the file is saved again. The exception would be caught and the user of the library might be unaware of the issue (the only way to notice would be to subscribe to the notification event of the reader and see an error message). Then saving the file again will cause the Prototype1b section to be missing.

[DomCR]

First of, thank you both for the effort and implication with the project! I really appreciate all the work that you are doing.

Now for the review of this PR, I've posted a few comments regarding the format and structure, format and architecture of the code, please feel free to discuss or propose any alternatives.

For the reading capability I would not be able to do a proper review giving that it will take some time to jump on the documentation and review bit by bit the reading process, so seeing that you both have run real case tests and I'll do the same with my files, if nothing crashes I'll consider the functionality valid to go into master.

To ensure that there are no major issues to the reading process please take in special account this comment:
#1154 (comment)
And add the same logic into the DxfReader for the current implementation so both are safe from crashing.

Again, thanks a lot for the amazing work!