Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
QuickLookStep currently reports
Failed to load STEP filefor files that containan auxiliary
GEOMETRICALLY_BOUNDED_SURFACE_SHAPE_REPRESENTATION, even when thesame file also contains supported B-Rep geometry.
The root cause is a Foxtrot panic while traversing that representation. The FFI
panic guard converts the panic to
false, so QuickLookStep can only surface thegeneric load error.
This draft updates the bundled Foxtrot revision to the narrow fix proposed in
Formlabs/foxtrot#25 and rebuilds
the tracked universal static library from that exact source revision.
Changes
de733262627f8a48ae882966325e144497d805a4;can fetch the pinned gitlink;
libfoxtrot_universal.afor both Apple Silicon and Intel Macs;C ABI.
Dependency / merge note
This PR is intentionally a draft while Formlabs/foxtrot#25 is under review.
It should not merge with the contributor fork URL. After the Foxtrot change is
merged, this branch should restore
.gitmodulesto the Formlabs HTTPS URL, pinthe submodule to the upstream merge commit, and rebuild the static library once
more from that final revision.
User impact
STEP files with supported solid geometry plus auxiliary bounded curve
representations can load instead of failing the entire preview. The change does
not add tessellation support for the auxiliary curve representation itself; it
only prevents that unsupported, non-solid representation from aborting supported
B-Rep triangulation.
Verification
steptests and the new sanitized triangulation regression test.foxtrot_ffiwith the repository lockfile for bothaarch64-apple-darwinandx86_64-apple-darwin._foxtrot_load_stepand_foxtrot_free_mesh.cuboid.step: 24 vertices / 12 triangles;cube_hole.step: 276 vertices / 268 triangles;load_failedbehavior with the previous library, thenloaded the same private real-world file successfully with the rebuilt library:
3,950,886 vertices / 3,042,400 triangles, with finite positions, valid bounds,
and in-range indices. The private file is not included in this PR.
thumbnail extension all link as universal
x86_64+arm64binaries.Known limitation
The private stress model still produces 36 non-finite normal components among
approximately 11.9 million normal components. Positions, bounds, and indices are
valid. This is an existing Foxtrot geometry-quality limitation outside the
panic fix and is not represented by the sanitized regression fixture.