diff --git a/TRANSFORMATION_DOCS.md b/TRANSFORMATION_DOCS.md index 773f27fe..4d3ff508 100644 --- a/TRANSFORMATION_DOCS.md +++ b/TRANSFORMATION_DOCS.md @@ -10,7 +10,7 @@ This document explains what the tool does and how to configure it. Transformation targets structural files containing positional node data: frames, suspension arms, subframes, chassis rails. The file must have a `nodes` section with the `["id", "posX", "posY", "posZ"]` header. -It is not intended for body files, engine files, gauges, interior parts, or any file where nodes have semantic names like `int_strsl`, `dshsl`, `e1`, `cam`. Those names carry meaning that transformation does not understand and will overwrite. +It is not intended for any file where nodes have semantic names like `int_strsl`, `dshsl`, `e1`, `cam`, which is common in engine files, gauges and interior parts. Those names carry meaning that transformation does not understand and will overwrite. --- @@ -34,30 +34,113 @@ This tells the tool that when it would generate a prefix derived from `rl_f`, us ## Configuration -Transformation reads `.jbeam-edit.yaml` in the working directory if present. Without it, built-in defaults are used. Create this file to override any parameter: +Transformation reads `.jbeam-edit.yaml` in the working directory if present. Without it, built-in defaults are used. Create this file to override any parameter. -```yaml -y-sorting-threshold: 0.05 -support-threshold: 96 -max-support-coordinates: 3 - -x-group-breakpoints: - - breakpoint: ">= 0.09" - vertex-type: LeftTree - - breakpoint: "> -0.09" - vertex-type: MiddleTree - - breakpoint: "<= -0.09" - vertex-type: RightTree -``` +[`examples/jbeam-edit.yaml`](examples/jbeam-edit.yaml) is a working one to copy. It is the config the test suite transforms every example file with, so it cannot drift out of date the way a block quoted here would. Parameter reference: -| Key | Default | Description | -|--------------------------|---------|--------------------------------------------------------------------------------| -| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | -| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | -| `max-support-coordinates`| 3 | Maximum number of support node candidates examined per spatial group | -| `x-group-breakpoints` | (above) | Rules that map X coordinate to Left, Middle, or Right | +| Key | Default | Description | +|---------------------------|---------|----------------------------------------------------------------------------------| +| `y-sorting-threshold` | 0.05 | Y distance (meters) below which two nodes are treated as the same depth band | +| `x-sorting-threshold` | off | X distance (meters) below which two nodes in one Y band count as the same column | +| `support-threshold` | 96 | Minimum beam count as a percentage of group size to classify a node as support | +| `max-support-coordinates` | 3 | Maximum number of support node candidates examined per spatial group | +| `x-group-breakpoints` | ±0.09 | Rules that map X coordinate to Left, Middle, or Right (see Left, Middle, Right) | + +`x-sorting-threshold` is off unless you set it, and `off` is the only word it +accepts besides a distance. There is no number that turns it off: `0` gives +every node its own column, which is the most column sorting rather than none. + +### Picking the two sorting thresholds + +Both settings are distances in meters, so `0.05` means 5 cm. + +They also work the same way, and it is worth knowing how. The tool goes through +the nodes in order and starts a new group as soon as a node is at least the +threshold away from **the first node of the group it is currently filling**. It +does not compare each node to the one right before it. That is deliberate: a +long gentle slope would otherwise chain together into one enormous group. + +**Start with `y-sorting-threshold`.** It decides how much front to back +variation still counts as the same row of nodes. When you place a row across a +panel the nodes are never at exactly the same Y, and without a threshold a +millimetre of difference would decide the order, which would also make the two +sides of the car come out differently. + +The number you want is the depth of one row, not the space between rows. The +default 5 cm suits a row you placed carefully on a flat face. A curved panel +needs more, because the row follows the curve. + +[`examples/regression_jbeam/y-sorting-repro.jbeam`](examples/regression_jbeam/y-sorting-repro.jbeam) +shows what that looks like. Its five frontmost left side nodes are one row as +far as the modeller is concerned, but they cover a fair bit of depth: + +| Measurement | Value | +|-----------------------------------------------|-------| +| Depth of the front row, -1.967 back to -1.791 | 0.176 | +| Front row's first node back to the next row's | 0.323 | +| Space between the two rows, -1.791 to -1.644 | 0.147 | +| Biggest step inside the front row | 0.138 | + +Anything above 0.176 and up to 0.323 keeps that row together, so 0.31 is a +comfortable pick and it is what the regression test uses. The default 0.05 +splits the row in two. Note the trap again: the space between the rows is 0.147, +barely more than the 0.138 step inside the row, so a threshold picked from the +space between rows lands in the wrong place. + +| What you see | What to do | +|---------------------------------------------|----------------------------------------------| +| One row comes out ordered front to back | Raise it, the row is deeper than you thought | +| Nodes at clearly different depths are mixed | Lower it | + +**Then `x-sorting-threshold`.** It does the same thing sideways, inside each +row. Turn it on if the file zigzags: the heights climb, drop back down and climb +again. That happens when one row covers two vertical columns of nodes, say the +nose face of a panel and the fender beside it, and the tool has nothing but +height to go on. + +Here is the part that trips people up. **The number you want is the width of a +column, not the space between the columns.** Because the tool measures from the +first node of a group, the threshold has to be a bit wider than your widest +column, and no wider than the step from one column's innermost node across to +the next column's innermost node. + +Those same five nodes show it. Once they are in one row, the inner column sits +at X 0.780, 0.920 and 0.953, and the outer one at 0.998 and 1.036: + +| Measurement | Value | +|------------------------------------------------|-------| +| Width of the inner column, 0.780 out to 0.953 | 0.173 | +| Inner column's innermost across to the outer's | 0.218 | +| Space between the two columns, 0.953 to 0.998 | 0.045 | +| Biggest step inside the inner column | 0.140 | + +So anything above 0.173 and up to 0.218 does the job, and 0.2 is the obvious +pick. Notice that the space between the columns is only 0.045, smaller than a +step inside the inner column. Set 0.045 and the tool splits the inner column in +two, leaves 0.780 on its own, and you get a third wrong order rather than the +right one. The space between the columns is the number that looks right, so it +is worth measuring the column itself instead. + +To find the number for your own vehicle, transform once with the setting off, +find a spot where the heights zigzag, and read the X values of those rows. The +two columns separate by eye. Take the first and last X of the wider column, +subtract, and pick something a little above that. Transform again and the zigzag +should be gone. + +If it still looks wrong: + +| What you see | What to do | +|--------------------------------------------|----------------------------------------------| +| Nothing changed at all | The threshold is too big, try a smaller one | +| Runs of one or two nodes, still zigzagging | The threshold is too small, try a bigger one | +| Still zigzagging whatever you set | Raise `y-sorting-threshold` first, see below | + +That last one is worth checking early, because no X value can fix it. Columns +only exist inside a row, so if your two columns sit further apart front to back +than `y-sorting-threshold` allows, they never end up in the same row and +`x-sorting-threshold` never gets to look at them together. Three ways this file can fail without saying much: @@ -160,6 +243,8 @@ Nodes within each group are sorted by three coordinates in order: A band starts at the node that opened it, not at the previous node, so a long run of small steps cannot chain into one band much wider than the threshold. +With `x-sorting-threshold` set, each Y band is banded a second time before step 2, by X and by the same opener rule. Nodes in the same Y band but different columns then come out one column at a time, lower node first within each. Without it a band that spans two columns is ordered by height alone, so the output climbs one column, jumps to the other and comes back. That is what the setting is for: on a body panel the nose face and the fender beside it can sit at heights that interleave, and no Y threshold separates them because they are at the same depth. + The threshold is there because nodes you placed at one depth are rarely at exactly the same Y. Without it, a millimetre of difference would order them by Y instead of by height, and the two sides of a symmetric vehicle would come out in different orders. --- @@ -209,7 +294,7 @@ Without a filename argument, all `.jbeam` files in the directory are validated. ## Limitations -**Body files are not supported.** Body files mix structural nodes with semantically named nodes (`int_strsl`, `dshsl`, `rm_*`). Transformation renames all nodes without distinction. An `excludePrefixes` config option is planned to protect named nodes from being renamed. +**Named nodes are not protected.** Transformation renames every node in the file without distinction, so a file that mixes structural nodes with semantically named ones (`int_strsl`, `dshsl`, `rm_*`) loses those names. Check a file for named nodes before transforming it. An `excludePrefixes` config option is planned to protect them. **The transformation feature is experimental.** It is not included in the standard release binary. To use it, you need to build from source with the `transformation` flag enabled, or download a build that explicitly includes it. diff --git a/examples/regression_jbeam/y-sorting-repro.jbeam b/examples/regression_jbeam/y-sorting-repro.jbeam index 2e3f2f13..87e9ec38 100644 --- a/examples/regression_jbeam/y-sorting-repro.jbeam +++ b/examples/regression_jbeam/y-sorting-repro.jbeam @@ -2,10 +2,16 @@ "testpart":{ "nodes":[ ["id", "posX", "posY", "posZ"], - // Synthetic regression-test fixture for issue #214, not vetted by - // the jbeam maintainer and not intended as a demo/example. + // Synthetic regression-test fixture, not vetted by the jbeam + // maintainer and not intended as a demo/example. // Real node positions from a gen4-style body file, reduced to just - // the left side, kept because this spacing reproduces a real bug. + // the left side, kept because this spacing reproduces two real bugs. + // Issue #214: at y-sorting-threshold 0.1 the frontmost node sorts to + // the back of its group. + // X columns: the five frontmost nodes sit in two vertical columns + // that Y and Z cannot separate, so a band sorted by Z alone climbs + // one column, jumps to the other and comes back. + // Change these positions and both tests are measuring something else. ["nl0", 0.953, -1.967, 0.122], ["nl2", 0.92, -1.953, 0.439], ["nl4", 0.78, -1.815, 0.719], diff --git a/jbeam-edit.cabal b/jbeam-edit.cabal index f3e0b62c..8c5b6dba 100644 --- a/jbeam-edit.cabal +++ b/jbeam-edit.cabal @@ -312,6 +312,7 @@ test-suite jbeam-edit-transformation-test main-is: Spec.hs hs-source-dirs: test-extra/transformation other-modules: + Spec.Config Spec.Helpers Spec.Regression Paths_jbeam_edit diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index f3b8ff1d..93299460 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -186,34 +186,18 @@ moveSupportVertices protectedNames newNames tfCfg connMap vsPerType = , count >= thrCount ] - brks = xGroupBreakpoints tfCfg - - assignSupportNames = assignNames newNames brks SupportTree - vertexForest :: VertexForest vertexForest = case NE.nonEmpty supportVertices of Nothing -> M.empty Just vs -> - M.singleton - SupportTree - ( OMap1.singleton - ( SupportKey - , VertexTree - [sideComment SupportTree] - ( let prefix = vertexPrefix newNames brks SupportTree - sorted = NE.sortBy (on compare $ vY . aVertex) vs - (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted - bandSortedPairs = - NE.sortBy - (compareAV prefix SupportTree) - bandIndices - bandSortedVertices = NE.map snd bandSortedPairs - (_, renamedVertices') = mapAccumL assignSupportNames M.empty bandSortedVertices - in renamedVertices' - ) + let supportTree = VertexTree [sideComment SupportTree] vs + sortedSupportTree = sortVertices SupportTree newNames tfCfg supportTree + in M.singleton + SupportTree + ( OMap1.singleton + (SupportKey, sortedSupportTree) ) - ) supportVertexNames = foldr (S.insert . anVertexName) S.empty supportVertices @@ -436,21 +420,36 @@ assignNames newNames brks treeType prefixMap av = firstY :: NonEmpty AnnotatedVertex -> Scientific firstY = vY . aVertex . NE.head +firstX :: NonEmpty (Int, AnnotatedVertex) -> Scientific +firstX = vX . aVertex . snd . NE.head + indexBand - :: TransformationConfig + :: Scientific + -> (a -> Scientific) -> (Int, Scientific) - -> AnnotatedVertex - -> ((Int, Scientific), (Int, AnnotatedVertex)) -indexBand tfCfg (bandIndex, bandY) av = - let nodeY = vY (aVertex av) + -> a + -> ((Int, Scientific), (Int, a)) +indexBand thr f (bandIndex, bandY) av = + let nodeY = f av distance = abs (nodeY - bandY) - thr = ySortingThreshold tfCfg in if distance >= thr then ((bandIndex + 1, nodeY), (bandIndex + 1, av)) else ((bandIndex, bandY), (bandIndex, av)) +columnBandVertices + :: Scientific + -> NonEmpty (Int, AnnotatedVertex) + -> NonEmpty ((Int, Int), AnnotatedVertex) +columnBandVertices thr vs = + let bandGroups = NE.groupBy1 (on (==) fst) vs + assignColumnBand group = + let sorted = NE.sortBy (on compare $ vX . aVertex . snd) group + (_, bandIndices) = mapAccumL (indexBand thr (vX . aVertex . snd)) (0, firstX sorted) sorted + in NE.map (\(xBand, (yBand, vertex)) -> ((yBand, xBand), vertex)) bandIndices + in sconcat $ NE.map assignColumnBand bandGroups + sortVertices :: VertexTreeType -> UpdateNamesMap @@ -459,13 +458,25 @@ sortVertices -> VertexTree sortVertices treeType newNames tfCfg (VertexTree comments vertices) = let brks = xGroupBreakpoints tfCfg + thr = ySortingThreshold tfCfg sorted = NE.sortBy (on compare $ vY . aVertex) vertices - (_, bandIndices) = mapAccumL (indexBand tfCfg) (0, firstY sorted) sorted - bandSortedAnnotated = - NE.map snd $ - NE.sortBy (compareAV (vertexPrefix newNames brks treeType) treeType) bandIndices + (_, bandIndices) = mapAccumL (indexBand thr (vY . aVertex)) (0, firstY sorted) sorted + + renameVertices + :: Ord a => NonEmpty (a, AnnotatedVertex) -> NonEmpty AnnotatedVertex + renameVertices banded = + let columnSortedAnnotated = + NE.map snd $ + NE.sortBy + (compareAV (vertexPrefix newNames brks treeType) treeType) + banded + in snd $ + mapAccumL (assignNames newNames brks treeType) M.empty columnSortedAnnotated + renamedGroups = - snd $ mapAccumL (assignNames newNames brks treeType) M.empty bandSortedAnnotated + case xSortingThreshold tfCfg of + Nothing -> renameVertices bandIndices + Just xThr -> renameVertices (columnBandVertices xThr bandIndices) in VertexTree comments renamedGroups updateVerticesInNode diff --git a/src-extra/transformation/JbeamEdit/Transformation/Config.hs b/src-extra/transformation/JbeamEdit/Transformation/Config.hs index 6579da3e..354c61f5 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/Config.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/Config.hs @@ -2,6 +2,7 @@ module JbeamEdit.Transformation.Config ( loadTransformationConfig, + decodeConfig, transformationConfigFile, applyOperator, newTransformationConfig, @@ -25,6 +26,7 @@ import Data.Yaml ( Object, ParseException (..), Parser, + Value (..), decodeEither', prettyPrintParseException, ) @@ -44,7 +46,10 @@ import JbeamEdit.IOUtils import JbeamEdit.Transformation.Types (VertexTreeType (..)) import Numeric.Natural (Natural) import System.OsPath -import Text.Read +import Text.Read (readMaybe) + +defaultXSortingThreshold :: Maybe Scientific +defaultXSortingThreshold = Nothing defaultSortingThreshold :: Scientific defaultSortingThreshold = 0.05 @@ -65,6 +70,7 @@ defaultBreakpoints = data TransformationConfig = TransformationConfig { ySortingThreshold :: Scientific + , xSortingThreshold :: Maybe Scientific , xGroupBreakpoints :: XGroupBreakpoints , supportThreshold :: Scientific , maxSupportCoordinates :: Natural @@ -75,6 +81,7 @@ newTransformationConfig :: TransformationConfig newTransformationConfig = TransformationConfig defaultSortingThreshold + defaultXSortingThreshold defaultBreakpoints defaultSupportThreshold defaultMaxSupportCoordinates @@ -134,10 +141,29 @@ parseSupportThreshold o = do fail "'support-threshold' must be a percentage value of 1 or higher (e.g., 80 or 80.8). Values below 1 (e.g., 0.80) are not allowed." +{- | Unquoted 'off' reaches this as a boolean, because that is how YAML +resolves it, so the string case alone would only catch the quoted spelling. +-} +parseXSortingThreshold :: Object -> Parser (Maybe Scientific) +parseXSortingThreshold o = do + thr <- o .:? "x-sorting-threshold" + case thr of + Nothing -> pure defaultXSortingThreshold + Just Null -> pure defaultXSortingThreshold + Just (Bool False) -> pure defaultXSortingThreshold + Just (String "off") -> pure defaultXSortingThreshold + Just (Number number) -> pure (Just number) + Just _ -> failWithMessage + where + failWithMessage = + fail + "'x-sorting-threshold' must be a distance in meters (e.g., 0.2 for 20 cm), or 'off' to leave the column sorting out. Omitting the key does the same as 'off'." + instance FromJSON TransformationConfig where parseJSON = withObject "TransformationConfig" $ \o -> TransformationConfig <$> o .:? "y-sorting-threshold" .!= defaultSortingThreshold + <*> parseXSortingThreshold o <*> o .:? "x-group-breakpoints" .!= defaultBreakpoints <*> parseSupportThreshold o <*> o .:? "max-support-coordinates" .!= defaultMaxSupportCoordinates diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index ce7067c5..201a0b7c 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -20,6 +20,7 @@ import JbeamEdit.Transformation.BeamExtraction (beamInKnownSet) import JbeamEdit.Transformation.BeamValidation import JbeamEdit.Transformation.Config import JbeamEdit.Transformation.Types (Beam) +import Spec.Config (configParsingSpec) import Spec.Helpers (parseJbeamFile) import Spec.Regression import System.Directory (getDirectoryContents) @@ -153,10 +154,12 @@ main = hspec $ do mapM_ (testInputFile "cfg-example" tfConfig) inputFiles mapM_ (testFixedPoint "cfg-default" newTransformationConfig) inputFiles mapM_ (testFixedPoint "cfg-example" tfConfig) inputFiles + configParsingSpec beamValidationSpec supportRenameIdempotencySpec letterEndingNodesSpec ySortingBandingSpec + xColumnSortingSpec metadataAcrossTreesSpec metadataPreservedSpec triangleMetadataSpec diff --git a/test-extra/transformation/Spec/Config.hs b/test-extra/transformation/Spec/Config.hs new file mode 100644 index 00000000..5394401f --- /dev/null +++ b/test-extra/transformation/Spec/Config.hs @@ -0,0 +1,82 @@ +{- | The config parser, read through `decodeConfig` because that is the entry +point `loadTransformationConfig` uses. A rejected config is not a loud +failure: the loader prints the message and returns the defaults, so the run +goes on to rewrite every neighbouring file with thresholds the user never +asked for. That is what makes these worth testing at all. +-} +module Spec.Config ( + configParsingSpec, +) where + +import Data.ByteString.Lazy qualified as LBS +import Data.Either (isLeft) +import Data.Text (Text) +import Data.Text qualified as T +import Data.Text.Encoding (encodeUtf8) +import JbeamEdit.Transformation.Config +import Test.Hspec + +parseField :: (TransformationConfig -> a) -> Text -> Either Text a +parseField field = + fmap field . decodeConfig . LBS.fromStrict . encodeUtf8 + +{- | Only the keys that predate the column pass. Leaving +`x-sorting-threshold` out keeps this source parseable, so the spec below +fails only for the reason it names. + +`support-threshold` is carried by every source here because it is the one +key the parser demands. +-} +olderThresholds :: Text +olderThresholds = + T.unlines + [ "y-sorting-threshold: 0.04" + , "support-threshold: 20" + , "max-support-coordinates: 3" + ] + +configParsingSpec :: Spec +configParsingSpec = describe "the transformation config parser" $ do + it "reads a bare YAML number for the keys that have always taken one" $ do + parseField ySortingThreshold olderThresholds `shouldBe` Right 0.04 + parseField supportThreshold olderThresholds `shouldBe` Right 20 + parseField maxSupportCoordinates olderThresholds `shouldBe` Right 3 + + it "reads a bare YAML number for x-sorting-threshold" $ + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: 0.2\n") + `shouldBe` Right (Just 0.2) + + it "reads a config that leaves support-threshold out" $ do + pendingWith + "support-threshold is the only key read with .: rather than .:?, so a \ + \config that omits it is rejected outright and the loader falls back to \ + \every default instead. The key is meant to have a default like the \ + \others, and this spec goes green with the change that gives it one." + parseField supportThreshold "y-sorting-threshold: 0.04\n" + `shouldBe` Right defaultSupportThreshold + + it "leaves the column pass off when x-sorting-threshold is absent" $ + parseField xSortingThreshold "support-threshold: 20\n" `shouldBe` Right Nothing + + it "reads off as leaving the column pass out, quoted or not" $ do + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: off\n") + `shouldBe` Right Nothing + parseField + xSortingThreshold + (olderThresholds <> "x-sorting-threshold: \"off\"\n") + `shouldBe` Right Nothing + + it "rejects an x-sorting-threshold that is neither a number nor off" $ + parseField xSortingThreshold (olderThresholds <> "x-sorting-threshold: soon\n") + `shouldSatisfy` isLeft + + it "accepts a support-threshold of exactly 1" $ + parseField supportThreshold "support-threshold: 1\n" `shouldBe` Right 1 + + it "rejects a support-threshold below 1" $ + parseField supportThreshold "support-threshold: 0.8\n" `shouldSatisfy` isLeft + + it "gives every default for an empty file" $ do + parseField ySortingThreshold "" `shouldBe` Right defaultSortingThreshold + parseField supportThreshold "" `shouldBe` Right defaultSupportThreshold + parseField xSortingThreshold "" `shouldBe` Right Nothing diff --git a/test-extra/transformation/Spec/Helpers.hs b/test-extra/transformation/Spec/Helpers.hs index 07ae87d4..2b8b944f 100644 --- a/test-extra/transformation/Spec/Helpers.hs +++ b/test-extra/transformation/Spec/Helpers.hs @@ -2,6 +2,7 @@ module Spec.Helpers ( parseJbeamFile, vertexPositionsInOrder, + vertexCoordinatesInOrder, vertexCoordinates, effectiveMetaByCoordinate, metaNumber, @@ -53,6 +54,25 @@ vertexPositionsInOrder topNode = , Just (Number yNum) <- [inner V.!? 2] ] +{- | Every vertex coordinate in a top node's "nodes" section, in the order +`transform` wrote them out. Use this where the defect is about which vertex +ended up where, rather than about which vertices survived. +-} +vertexCoordinatesInOrder :: Node -> [(Double, Double, Double)] +vertexCoordinatesInOrder topNode = + case NP.queryNodes nodesQuery topNode >>= NP.expectArray nodesQuery of + Left _ -> [] + Right rows -> + [ (realToFrac (nvValue x), realToFrac (nvValue y), realToFrac (nvValue z)) + | row <- V.toList rows + , Just inner <- [expectArray row] + , Just (String name) <- [inner V.!? 0] + , name /= "id" + , Just (Number x) <- [inner V.!? 1] + , Just (Number y) <- [inner V.!? 2] + , Just (Number z) <- [inner V.!? 3] + ] + {- | Every vertex coordinate in a top node's "nodes" section. Positions survive renaming, so they identify a vertex across a transform. -} diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index ff041164..71aaf418 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -9,6 +9,7 @@ module Spec.Regression ( ySortingBandingSpec, metadataAcrossTreesSpec, metadataPreservedSpec, + xColumnSortingSpec, ) where import Data.Map qualified as M @@ -171,3 +172,51 @@ metadataPreservedSpec = ] M.keys metaAfter `shouldBe` M.keys metaBefore changed `shouldBe` [] + +{- | The same gen4-style left-side positions as the y-sorting fixture, read +for a different defect. Its five frontmost nodes sit in two vertical +columns: an inner one at X 0.780/0.920/0.953 and an outer one at X +0.998/1.036, the nose face and the fender beside it. Y and Z interleave +between the two columns, so no y-sorting-threshold can separate them; only +X can. Sorting a band by Z alone therefore climbs one column, jumps to the +other and comes back, which is what the jbeam maintainer marked up on his +render. + +The Y threshold here is 0.31 rather than the default because that is what puts +all five in one band, which is where the defect lives. It is not free choice: +between 0.153 and 0.16 the Y bands land on exactly the two columns, and this +assertion passes with nothing fixed at all. + +`xSortingThreshold` has to be set explicitly because it has no default. There is +no number that means off (0 gives every vertex its own band, which is the most +X sorting rather than none), so the field is optional and absent means the pass +does not run at all. +-} +xColumnSortingSpec :: Spec +xColumnSortingSpec = + describe "vertices in one Y band but different X columns" $ do + it "keeps each column contiguous instead of interleaving them" $ do + let inner = [(0.953, -1.967, 0.122), (0.92, -1.953, 0.439), (0.78, -1.815, 0.719)] + outer = [(1.036, -1.807, 0.125), (0.998, -1.791, 0.473)] + topNode <- parseJbeamFile ySortingReproFixture + withColumnSorting topNode $ \resultNode -> + take 5 (leftGroup (vertexCoordinatesInOrder resultNode)) + `shouldBe` inner ++ outer + + it "is a fixed point: a second transform moves nothing further" $ do + topNode <- parseJbeamFile ySortingReproFixture + withColumnSorting topNode $ \resultNode -> + withColumnSorting resultNode $ \againNode -> + vertexCoordinatesInOrder againNode + `shouldBe` vertexCoordinatesInOrder resultNode + where + leftGroup = filter (\(x, _, _) -> x >= 0.09) + columnSortingConfig = + newTransformationConfig + { ySortingThreshold = 0.31 + , xSortingThreshold = Just 0.2 + } + withColumnSorting node assert = + case transform M.empty columnSortingConfig node of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right (_, _, _, resultNode) -> assert resultNode