-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix PolyfaceAuxData index blocking flatbuffer format / Reorganize test data #6726
Conversation
…xData index formats
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only reviewed the last commit where you extended getPointCurveClosestApproachXYNewton
to support LineString. Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving but consider putting the massive code reorganization changes into a separate PR or at the very least changing the PR title to describe what's actually being changed.
Thanks. Yeah, this one got out of control. Will definitely do next time. |
Addresses #6583.
TypeScript FlatBuffer serialization of
PolyfaceAuxData
indices lacked the 0-blocking of the otherPolyface
index arrays, which led to crashes during mesh processing. Now TypeScript FlatBuffer serialization outputs the correct index blocking, and both TypeScript and native FlatBuffer APIs read both old and new PolyfaceAuxData index formats. The old format is detected using heuristics.Although there is a similar imodel-native PR, the fixes in each repo are independent of the other.
Also:
Polyface
indices, which havenumPerFace > 2
and are only written by the native FlatBuffer API. Move index array reblocking logic toSerializationHelpers
for common usage.Polyface
variations innumPerFace
value,PolyfaceAuxData
format, and all combinations thereof.IndexedMeshProps
was incomplete and used the wrong label forTaggedNumericDataProps
.PolyfaceAuxData
and use them in the JSON deserializer.assert(!"foo")
idiomIndexedPolyface.terminateFacet
so that it can be called once after all facets are added, instead of after each facet. This new index validation caught at least one bug in a mesh created for aPolyfaceQuery
test.foo?: bar
overfoo: bar | undefined
PolyfaceData.compress
now compressesPolyfaceAuxData
if exactly oneAuxChannelData
is presentSphere
ctor now validates latitude sweep to avoid common user error.Sphere.createEllipsoid
was erroneously capturing inputs.Checker.testDefined
andChecker.testUndefined
to simplify many unit tests. The linter pointed out the simplifications to the tests.GeometryCoreTestIO
: add helper functions for tests that perform FlatBuffer/JSON <-> file/GeometryQuery
conversionsPolyfaceAuxData
. In particular emphasize that the indices must have the same blocking as the otherPolyface
index arrays.