From f5b70f31548cfd30af6356ba8b1d9aabf6fcc287 Mon Sep 17 00:00:00 2001 From: Karl Ostmo Date: Sun, 18 Aug 2024 15:33:24 -0700 Subject: [PATCH] fix positioning bug with negative coordinates --- .../00-ORDER.txt | 3 +- .../sequential-placement.yaml | 77 +++++++++++++++++++ .../circle-and-crosses.yaml | 2 +- .../Scenario/Topography/Structure/Assembly.hs | 23 +++--- .../Scenario/Topography/Structure/Overlay.hs | 7 +- test/integration/Main.hs | 9 ++- test/standalone-topography/src/Lib.hs | 11 +-- test/standalone-topography/src/Main.hs | 59 ++++++++------ 8 files changed, 147 insertions(+), 44 deletions(-) create mode 100644 data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml diff --git a/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt b/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt index 8580fa46e..ed9116ef4 100644 --- a/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt +++ b/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt @@ -1,3 +1,4 @@ nonoverlapping-structure-merge.yaml root-map-expansion.yaml -structure-composition.yaml \ No newline at end of file +structure-composition.yaml +sequential-placement.yaml \ No newline at end of file diff --git a/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml b/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml new file mode 100644 index 000000000..7a7bdcab0 --- /dev/null +++ b/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml @@ -0,0 +1,77 @@ +version: 1 +name: Flipped structure placement +author: Karl Ostmo +description: | + Sequentially place structures that are larger than the map + with flipped orientation. +robots: + - name: base + dir: north +known: [boulder, log, pixel (R), pixel (G), pixel (B), gold] +world: + structures: + - name: reddish + structure: + mask: '.' + palette: + 'x': [stone, pixel (R)] + map: | + xx + x. + - name: greenish + structure: + mask: '.' + palette: + 'x': [stone, pixel (G)] + map: | + xx + x. + - name: bluish + structure: + mask: '.' + palette: + 'x': [stone, pixel (B)] + map: | + xx + x. + - name: goldish + structure: + mask: '.' + palette: + 'x': [stone, gold] + map: | + xx + x. + - name: block + structure: + mask: '.' + palette: + 'x': [ice, log] + placements: + - src: greenish + orient: + flip: true + offset: [-3, 2] + - src: reddish + offset: [-6, -1] + - src: goldish + orient: + flip: true + offset: [3, 0] + - src: bluish + offset: [0, 1] + map: | + xxx + xx. + x.. + palette: + 'Ω': [grass, erase, base] + mask: '.' + placements: + - src: block + offset: [0, -1] + upperleft: [0, 0] + dsl: | + {grass} + map: | + Ω diff --git a/data/test/standalone-topography/circle-and-crosses.yaml b/data/test/standalone-topography/circle-and-crosses.yaml index e5f650bd1..dada1adfd 100644 --- a/data/test/standalone-topography/circle-and-crosses.yaml +++ b/data/test/standalone-topography/circle-and-crosses.yaml @@ -19,7 +19,7 @@ structures: fff placements: - src: beam - offset: [0, 3] + offset: [0, 0] - src: beam offset: [-3, -3] orient: diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs index 06a6e12ac..c78f1398c 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs @@ -22,7 +22,6 @@ import Data.Text qualified as T import Linear.Affine import Swarm.Game.Location import Swarm.Game.Scenario.Topography.Area -import Swarm.Game.Scenario.Topography.Grid import Swarm.Game.Scenario.Topography.Navigation.Waypoint import Swarm.Game.Scenario.Topography.Placement import Swarm.Game.Scenario.Topography.Structure @@ -42,14 +41,14 @@ overlaySingleStructure :: Either Text (MergedStructure (Maybe a)) overlaySingleStructure inheritedStrucDefs - (Placed p@(Placement _ pose@(Pose loc orientation)) ns) + (Placed p@(Placement _sName pose@(Pose loc orientation)) ns) (MergedStructure inputArea inputPlacements inputWaypoints) = do MergedStructure overlayArea overlayPlacements overlayWaypoints <- mergeStructures inheritedStrucDefs (WithParent p) $ structure ns let mergedWaypoints = inputWaypoints <> map (fmap $ placeOnArea overlayArea) overlayWaypoints mergedPlacements = inputPlacements <> map (placeOnArea overlayArea) overlayPlacements - mergedArea = overlayGridExpanded (gridContent inputArea) pose overlayArea + mergedArea = overlayGridExpanded inputArea pose overlayArea return $ MergedStructure mergedArea mergedPlacements mergedWaypoints where @@ -81,6 +80,8 @@ mergeStructures inheritedStrucDefs parentPlacement (Structure origArea subStruct map wrapPlacement $ filter (\(Placed _ ns) -> isRecognizable ns) overlays + -- NOTE: Each successive overlay may alter the coordinate origin. + -- We make sure this new origin is propagated to subsequent sibling placements. foldlM (flip $ overlaySingleStructure structureMap) (MergedStructure origArea wrappedOverlays originatedWaypoints) @@ -97,18 +98,22 @@ mergeStructures inheritedStrucDefs parentPlacement (Structure origArea subStruct -- * Grid manipulation overlayGridExpanded :: - Grid (Maybe a) -> + PositionedGrid (Maybe a) -> Pose -> PositionedGrid (Maybe a) -> PositionedGrid (Maybe a) overlayGridExpanded - inputGrid - (Pose loc orientation) - (PositionedGrid _ overlayArea) = - PositionedGrid origin inputGrid <> positionedOverlay + baseGrid + (Pose yamlPlacementOffset orientation) + -- NOTE: The '_childAdjustedOrigin' is the sum of origin adjustments + -- to completely assemble some substructure. However, we discard + -- this when we place a substructure into a new base grid. + (PositionedGrid _childAdjustedOrigin overlayArea) = + baseGrid <> positionedOverlay where reorientedOverlayCells = applyOrientationTransform orientation overlayArea - positionedOverlay = PositionedGrid loc reorientedOverlayCells + placementAdjustedByOrigin = gridPosition baseGrid .+^ (yamlPlacementOffset .-. origin) + positionedOverlay = PositionedGrid placementAdjustedByOrigin reorientedOverlayCells -- * Validation diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs index 86444ed5a..f666b7198 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs @@ -101,12 +101,11 @@ instance (Alternative f) => Semigroup (PositionedGrid (f a)) where mergedSize = computeMergedArea $ OverlayPair a1 a2 combinedGrid = zipGridRows mergedSize paddedOverlayPair - -- We subtract the base origin from the - -- overlay position, such that the displacement vector - -- will have: + -- We create a vector from the overlay position, + -- such that the displacement vector will have: -- \* negative X component if the origin must be shifted east -- \* positive Y component if the origin must be shifted south - originDelta@(V2 deltaX deltaY) = overlayLoc .-. baseLoc + originDelta@(V2 deltaX deltaY) = overlayLoc .-. origin -- Note that the adjustment vector will only ever have -- a non-negative X component (i.e. loc of upper-left corner must be shifted east) and -- a non-positive Y component (i.e. loc of upper-left corner must be shifted south). diff --git a/test/integration/Main.hs b/test/integration/Main.hs index 9f0fa9c8c..8253b8aa9 100644 --- a/test/integration/Main.hs +++ b/test/integration/Main.hs @@ -440,7 +440,14 @@ testScenarioSolutions rs ui key = [ testSolution Default "Testing/1535-ping/1535-in-range" , testSolution Default "Testing/1535-ping/1535-out-of-range" ] - , testGroup + , -- , testGroup + -- "Structure placement (#1780)" + -- [ testSolution Default "Testing/1780-structure-merge-expansion/nonoverlapping-structure-merge" + -- , testSolution Default "Testing/1780-structure-merge-expansion/root-map-expansion" + -- , testSolution Default "Testing/1780-structure-merge-expansion/structure-composition" + -- , testSolution Default "Testing/1780-structure-merge-expansion/sequential-placement" + -- ] + testGroup "Structure recognition (#1575)" [ testSolution Default "Testing/1575-structure-recognizer/1575-browse-structures" , testSolution Default "Testing/1575-structure-recognizer/1575-nested-structure-definition" diff --git a/test/standalone-topography/src/Lib.hs b/test/standalone-topography/src/Lib.hs index 1fa47d2c5..c73991d8e 100644 --- a/test/standalone-topography/src/Lib.hs +++ b/test/standalone-topography/src/Lib.hs @@ -32,8 +32,12 @@ parseStructures dataDir baseFilename = do dataDir "test/standalone-topography" baseFilename return $ forceEither $ left prettyPrintParseException eitherResult -compareToReferenceImage :: FilePath -> Assertion -compareToReferenceImage fileStem = do +compareToReferenceImage :: + -- | set this to update the golden tests + Bool -> + FilePath -> + Assertion +compareToReferenceImage refreshReferenceImage fileStem = do dataDir <- getDataDir parentStruct <- parseStructures dataDir $ fileStem <.> "yaml" let MergedStructure overlayArea _ _ = forceEither $ mergeStructures mempty Root parentStruct @@ -44,6 +48,3 @@ compareToReferenceImage fileStem = do else do decodedImg <- LBS.readFile referenceFilepath assertEqual "Generated image must equal reference image!" decodedImg encodedImgBytestring - where - -- Manually toggle this to update the golden tests - refreshReferenceImage = False diff --git a/test/standalone-topography/src/Main.hs b/test/standalone-topography/src/Main.hs index cd8438307..184e496be 100644 --- a/test/standalone-topography/src/Main.hs +++ b/test/standalone-topography/src/Main.hs @@ -4,33 +4,46 @@ -- SPDX-License-Identifier: BSD-3-Clause module Main where -import Test.Tasty (defaultMain, testGroup) +import Data.Proxy +import Data.Typeable (Typeable) +import Lib (compareToReferenceImage) +import Test.Tasty import Test.Tasty.HUnit (testCase) +import Test.Tasty.Options -import Lib +newtype UpdateGoldenTests = UpdateGoldenTests Bool + deriving (Eq, Ord, Typeable) + +instance IsOption UpdateGoldenTests where + parseValue = fmap UpdateGoldenTests . safeRead + defaultValue = UpdateGoldenTests False + optionName = return "refresh" + optionHelp = return "Should overwrite the golden test images" + optionCLParser = mkFlagCLParser mempty (UpdateGoldenTests True) main :: IO () main = do - defaultMain $ - testGroup - "Test structure assembly" - [ mkGroup - "Black and white" - [ "circle-and-crosses" - , "checkerboard" - ] - , mkGroup - "Color" - [ "rainbow" + defaultMainWithIngredients ingreds $ askOption $ \(UpdateGoldenTests shouldRefreshTests) -> + let doTest stem = + testCase (unwords ["Image equality:", stem]) $ + compareToReferenceImage shouldRefreshTests stem + + mkGroup title members = + testGroup title $ + map + doTest + members + in testGroup + "Test structure assembly" + [ mkGroup + "Black and white" + [ "circle-and-crosses" + , "checkerboard" + ] + , mkGroup + "Color" + [ "rainbow" + ] ] - ] where - doTest stem = - testCase (unwords ["Image equality:", stem]) $ - compareToReferenceImage stem - - mkGroup title members = - testGroup title $ - map - doTest - members + ingreds = includingOptions [Option (Proxy :: Proxy UpdateGoldenTests)] : defaultIngredients