From bbdba30b91a84f9d6fef6da371e7361fb310016a Mon Sep 17 00:00:00 2001
From: Alejandro Perea <alexpro820@gmail.com>
Date: Mon, 14 Feb 2022 16:01:46 +0100
Subject: [PATCH] Refactor chunk system (#157)

* Refactor chunk system
- Add `FiniteTileLayer` & `InfiniteTileLayer`
- Chunks are now stored by chunk position
- Add `InternalChunk` for holding mid-parse chunk info

* Fix tests

* Cleanup

* Make chunk constructor private

* Address PR comments

* Box chunk tiles

* Convert numeric types before multiplication
---
 examples/main.rs            | 26 +++++------
 examples/sfml/main.rs       | 25 ++++++-----
 src/layers/mod.rs           |  8 ++--
 src/layers/tile/finite.rs   | 18 ++++++--
 src/layers/tile/infinite.rs | 89 +++++++++++++++++++++++++++++++------
 src/layers/tile/mod.rs      | 32 +++++++------
 src/util.rs                 | 11 +++++
 tests/lib.rs                | 66 ++++++++++++++++-----------
 8 files changed, 193 insertions(+), 82 deletions(-)

diff --git a/examples/main.rs b/examples/main.rs
index 5379f50..f99bfb6 100644
--- a/examples/main.rs
+++ b/examples/main.rs
@@ -17,22 +17,25 @@ fn main() {
         print!("Layer \"{}\":\n\t", layer.data().name);
 
         match layer.layer_type() {
-            tiled::LayerType::TileLayer(layer) => match layer.data() {
-                tiled::TileLayerData::Finite(data) => println!(
+            tiled::LayerType::TileLayer(layer) => match layer {
+                tiled::TileLayer::Finite(data) => println!(
                     "Finite tile layer with width = {} and height = {}; ID of tile @ (0,0): {}",
-                    data.width(),
-                    data.height(),
-                    layer.get_tile(0, 0).unwrap().id
+                    data.data().width(),
+                    data.data().height(),
+                    data.get_tile(0, 0).unwrap().id
                 ),
-                tiled::TileLayerData::Infinite(data) => {
+                tiled::TileLayer::Infinite(data) => {
                     // This is prone to change! Infinite layers will be refactored before 0.10.0
                     // releases.
-                    println!("Infinite tile layer with {} chunks", data.chunks.len())
+                    println!(
+                        "Infinite tile layer; Tile @ (-5, 0) = {:?}",
+                        data.get_tile(-5, 0)
+                    )
                 }
             },
             tiled::LayerType::ObjectLayer(layer) => {
                 println!("Object layer with {} objects", layer.data().objects.len())
-            },
+            }
             tiled::LayerType::ImageLayer(layer) => {
                 println!(
                     "Image layer with {}",
@@ -42,12 +45,9 @@ fn main() {
                         None => "no image".to_owned(),
                     }
                 )
-            },
+            }
             tiled::LayerType::GroupLayer(layer) => {
-                println!(
-                    "Group layer with {} sublayers",
-                    layer.layers().len()
-                )
+                println!("Group layer with {} sublayers", layer.layers().len())
             }
         }
     }
diff --git a/examples/sfml/main.rs b/examples/sfml/main.rs
index 0dab7cd..989bd61 100644
--- a/examples/sfml/main.rs
+++ b/examples/sfml/main.rs
@@ -13,7 +13,7 @@ use sfml::{
     window::{ContextSettings, Key, Style},
 };
 use std::{env, path::PathBuf, time::Duration};
-use tiled::{FilesystemResourceCache, Map, TileLayer};
+use tiled::{FilesystemResourceCache, FiniteTileLayer, Map};
 use tilesheet::Tilesheet;
 
 /// A path to the map to display.
@@ -44,7 +44,13 @@ impl Level {
         let layers = map
             .layers()
             .filter_map(|layer| match &layer.layer_type() {
-                tiled::LayerType::TileLayer(l) => Some(generate_mesh(l, &tilesheet)),
+                tiled::LayerType::TileLayer(l) => Some(generate_mesh(
+                    match l {
+                        tiled::TileLayer::Finite(f) => f,
+                        tiled::TileLayer::Infinite(_) => panic!("Infinite maps not supported"),
+                    },
+                    &tilesheet,
+                )),
                 _ => None,
             })
             .collect();
@@ -58,15 +64,14 @@ impl Level {
 }
 
 /// Generates a vertex mesh from a tile layer for rendering.
-fn generate_mesh(layer: &TileLayer, tilesheet: &Tilesheet) -> QuadMesh {
-    let finite = match layer.data() {
-        tiled::TileLayerData::Finite(f) => f,
-        tiled::TileLayerData::Infinite(_) => panic!("Infinite maps not supported"),
-    };
-    let (width, height) = (finite.width() as usize, finite.height() as usize);
+fn generate_mesh(layer: &FiniteTileLayer, tilesheet: &Tilesheet) -> QuadMesh {
+    let (width, height) = (
+        layer.data().width() as usize,
+        layer.data().height() as usize,
+    );
     let mut mesh = QuadMesh::with_capacity(width * height);
-    for x in 0..width {
-        for y in 0..height {
+    for x in 0..width as i32 {
+        for y in 0..height as i32 {
             // TODO: `FiniteTileLayer` for getting tiles directly from finite tile layers?
             if let Some(tile) = layer.get_tile(x, y) {
                 let uv = tilesheet.tile_rect(tile.id);
diff --git a/src/layers/mod.rs b/src/layers/mod.rs
index 272b496..a7fe62d 100644
--- a/src/layers/mod.rs
+++ b/src/layers/mod.rs
@@ -2,7 +2,9 @@ use std::path::Path;
 
 use xml::attribute::OwnedAttribute;
 
-use crate::{error::TiledError, properties::Properties, util::*, Map, MapTilesetGid, MapWrapper, Color};
+use crate::{
+    error::TiledError, properties::Properties, util::*, Color, Map, MapTilesetGid, MapWrapper,
+};
 
 mod image;
 pub use image::*;
@@ -14,7 +16,7 @@ mod group;
 pub use group::*;
 
 #[derive(Clone, PartialEq, Debug)]
-pub enum LayerDataType {
+pub(crate) enum LayerDataType {
     TileLayer(TileLayerData),
     ObjectLayer(ObjectLayerData),
     ImageLayer(ImageLayerData),
@@ -41,7 +43,7 @@ pub struct LayerData {
     pub opacity: f32,
     pub tint_color: Option<Color>,
     pub properties: Properties,
-    pub layer_type: LayerDataType,
+    pub(crate) layer_type: LayerDataType,
 }
 
 impl LayerData {
diff --git a/src/layers/tile/finite.rs b/src/layers/tile/finite.rs
index 55e9eb0..69e8aa7 100644
--- a/src/layers/tile/finite.rs
+++ b/src/layers/tile/finite.rs
@@ -2,7 +2,7 @@ use xml::attribute::OwnedAttribute;
 
 use crate::{
     util::{get_attrs, XmlEventResult},
-    LayerTileData, MapTilesetGid, TiledError,
+    LayerTile, LayerTileData, MapTilesetGid, MapWrapper, TiledError,
 };
 
 use super::util::parse_data_line;
@@ -51,9 +51,9 @@ impl FiniteTileLayerData {
         })
     }
 
-    pub(crate) fn get_tile(&self, x: usize, y: usize) -> Option<&LayerTileData> {
-        if x < self.width as usize && y < self.height as usize {
-            self.tiles[x + y * self.width as usize].as_ref()
+    pub(crate) fn get_tile(&self, x: i32, y: i32) -> Option<&LayerTileData> {
+        if x < self.width as i32 && y < self.height as i32 && x >= 0 && y >= 0 {
+            self.tiles[x as usize + y as usize * self.width as usize].as_ref()
         } else {
             None
         }
@@ -69,3 +69,13 @@ impl FiniteTileLayerData {
         self.height
     }
 }
+
+pub type FiniteTileLayer<'map> = MapWrapper<'map, FiniteTileLayerData>;
+
+impl<'map> FiniteTileLayer<'map> {
+    pub fn get_tile(&self, x: i32, y: i32) -> Option<LayerTile> {
+        self.data()
+            .get_tile(x, y)
+            .and_then(|data| Some(LayerTile::from_data(data, self.map())))
+    }
+}
diff --git a/src/layers/tile/infinite.rs b/src/layers/tile/infinite.rs
index 1a50aac..d7b4dcb 100644
--- a/src/layers/tile/infinite.rs
+++ b/src/layers/tile/infinite.rs
@@ -3,15 +3,15 @@ use std::collections::HashMap;
 use xml::attribute::OwnedAttribute;
 
 use crate::{
-    util::{get_attrs, parse_tag, XmlEventResult},
-    LayerTileData, MapTilesetGid, TiledError,
+    util::{floor_div, get_attrs, parse_tag, XmlEventResult},
+    LayerTile, LayerTileData, MapTilesetGid, MapWrapper, TiledError,
 };
 
 use super::util::parse_data_line;
 
 #[derive(PartialEq, Clone)]
 pub struct InfiniteTileLayerData {
-    pub chunks: HashMap<(i32, i32), Chunk>,
+    chunks: HashMap<(i32, i32), Chunk>,
 }
 
 impl std::fmt::Debug for InfiniteTileLayerData {
@@ -39,33 +39,86 @@ impl InfiniteTileLayerData {
         let mut chunks = HashMap::<(i32, i32), Chunk>::new();
         parse_tag!(parser, "data", {
             "chunk" => |attrs| {
-                let chunk = Chunk::new(parser, attrs, e.clone(), c.clone(), tilesets)?;
-                chunks.insert((chunk.x, chunk.y), chunk);
+                let chunk = InternalChunk::new(parser, attrs, e.clone(), c.clone(), tilesets)?;
+                for x in chunk.x..chunk.x + chunk.width as i32 {
+                    for y in chunk.y..chunk.y + chunk.height as i32 {
+                        let chunk_pos = tile_to_chunk_pos(x, y);
+                        let relative_pos = (x - chunk_pos.0 * Chunk::WIDTH as i32, y - chunk_pos.1 * Chunk::HEIGHT as i32);
+                        let chunk_index = (relative_pos.0 + relative_pos.1 * Chunk::WIDTH as i32) as usize;
+                        let internal_pos = (x - chunk.x, y - chunk.y);
+                        let internal_index = (internal_pos.0 + internal_pos.1 * chunk.width as i32) as usize;
+
+                        chunks.entry(chunk_pos).or_insert_with(Chunk::new).tiles[chunk_index] = chunk.tiles[internal_index];
+                    }
+                }
                 Ok(())
             }
         });
 
         Ok(Self { chunks })
     }
+
+    pub(crate) fn get_tile(&self, x: i32, y: i32) -> Option<&LayerTileData> {
+        let chunk_pos = tile_to_chunk_pos(x, y);
+        self.chunks
+            .get(&chunk_pos)
+            .and_then(|chunk| {
+                let relative_pos = (
+                    x - chunk_pos.0 * Chunk::WIDTH as i32,
+                    y - chunk_pos.1 * Chunk::HEIGHT as i32,
+                );
+                let chunk_index = (relative_pos.0 + relative_pos.1 * Chunk::WIDTH as i32) as usize;
+                chunk.tiles.get(chunk_index).map(Option::as_ref)
+            })
+            .flatten()
+    }
+}
+
+fn tile_to_chunk_pos(x: i32, y: i32) -> (i32, i32) {
+    (
+        floor_div(x, Chunk::WIDTH as i32),
+        floor_div(y, Chunk::HEIGHT as i32),
+    )
 }
 
 #[derive(Debug, PartialEq, Clone)]
 pub struct Chunk {
-    pub x: i32,
-    pub y: i32,
-    pub width: u32,
-    pub height: u32,
-    tiles: Vec<Option<LayerTileData>>,
+    tiles: Box<[Option<LayerTileData>; Self::TILE_COUNT]>,
 }
 
 impl Chunk {
+    pub const WIDTH: u32 = 16;
+    pub const HEIGHT: u32 = 16;
+    pub const TILE_COUNT: usize = Self::WIDTH as usize * Self::HEIGHT as usize;
+
+    pub(crate) fn new() -> Self {
+        Self {
+            tiles: Box::new([None; Self::TILE_COUNT]),
+        }
+    }
+}
+
+#[derive(Debug, PartialEq, Clone)]
+struct InternalChunk {
+    /// The X coordinate of the top-left-most tile in the chunk.
+    /// Corresponds to the `x` attribute in the TMX format.
+    x: i32,
+    /// The Y coordinate of the top-left-most tile in the chunk.
+    /// Corresponds to the `y` attribute in the TMX format.
+    y: i32,
+    width: u32,
+    height: u32,
+    tiles: Vec<Option<LayerTileData>>,
+}
+
+impl InternalChunk {
     pub(crate) fn new(
         parser: &mut impl Iterator<Item = XmlEventResult>,
         attrs: Vec<OwnedAttribute>,
         encoding: Option<String>,
         compression: Option<String>,
         tilesets: &[MapTilesetGid],
-    ) -> Result<Chunk, TiledError> {
+    ) -> Result<Self, TiledError> {
         let ((), (x, y, width, height)) = get_attrs!(
             attrs,
             optionals: [],
@@ -75,12 +128,12 @@ impl Chunk {
                 ("width", width, |v: String| v.parse().ok()),
                 ("height", height, |v: String| v.parse().ok()),
             ],
-            TiledError::MalformedAttributes("layer must have a name".to_string())
+            TiledError::MalformedAttributes("chunk must have x, y, width & height attributes".to_string())
         );
 
         let tiles = parse_data_line(encoding, compression, parser, tilesets)?;
 
-        Ok(Chunk {
+        Ok(InternalChunk {
             x,
             y,
             width,
@@ -89,3 +142,13 @@ impl Chunk {
         })
     }
 }
+
+pub type InfiniteTileLayer<'map> = MapWrapper<'map, InfiniteTileLayerData>;
+
+impl<'map> InfiniteTileLayer<'map> {
+    pub fn get_tile(&self, x: i32, y: i32) -> Option<LayerTile> {
+        self.data()
+            .get_tile(x, y)
+            .and_then(|data| Some(LayerTile::from_data(data, self.map())))
+    }
+}
diff --git a/src/layers/tile/mod.rs b/src/layers/tile/mod.rs
index 49c06b0..a48608c 100644
--- a/src/layers/tile/mod.rs
+++ b/src/layers/tile/mod.rs
@@ -5,7 +5,7 @@ use xml::attribute::OwnedAttribute;
 use crate::{
     parse_properties,
     util::{get_attrs, parse_tag, XmlEventResult},
-    Gid, Map, MapTilesetGid, MapWrapper, Properties, Tile, TileId, TiledError, Tileset,
+    Gid, Map, MapTilesetGid, Properties, Tile, TileId, TiledError, Tileset,
 };
 
 mod finite;
@@ -61,7 +61,7 @@ impl LayerTileData {
 }
 
 #[derive(Debug, PartialEq, Clone)]
-pub enum TileLayerData {
+pub(crate) enum TileLayerData {
     Finite(FiniteTileLayerData),
     Infinite(InfiniteTileLayerData),
 }
@@ -102,13 +102,6 @@ impl TileLayerData {
 
         Ok((result, properties))
     }
-
-    pub(crate) fn get_tile(&self, x: usize, y: usize) -> Option<&LayerTileData> {
-        match &self {
-            Self::Finite(finite) => finite.get_tile(x, y),
-            Self::Infinite(_) => todo!("Getting tiles from infinite layers"),
-        }
-    }
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -137,12 +130,23 @@ impl<'map> LayerTile<'map> {
     }
 }
 
-pub type TileLayer<'map> = MapWrapper<'map, TileLayerData>;
+pub enum TileLayer<'map> {
+    Finite(FiniteTileLayer<'map>),
+    Infinite(InfiniteTileLayer<'map>),
+}
 
 impl<'map> TileLayer<'map> {
-    pub fn get_tile(&self, x: usize, y: usize) -> Option<LayerTile> {
-        self.data()
-            .get_tile(x, y)
-            .and_then(|data| Some(LayerTile::from_data(data, self.map())))
+    pub(crate) fn new(map: &'map Map, data: &'map TileLayerData) -> Self {
+        match data {
+            TileLayerData::Finite(data) => Self::Finite(FiniteTileLayer::new(map, data)),
+            TileLayerData::Infinite(data) => Self::Infinite(InfiniteTileLayer::new(map, data)),
+        }
+    }
+
+    pub fn get_tile(&self, x: i32, y: i32) -> Option<LayerTile> {
+        match self {
+            TileLayer::Finite(finite) => finite.get_tile(x, y),
+            TileLayer::Infinite(infinite) => infinite.get_tile(x, y),
+        }
     }
 }
diff --git a/src/util.rs b/src/util.rs
index e9d5127..44eff99 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -70,3 +70,14 @@ pub(crate) fn get_tileset_for_gid(
         .rev()
         .find(|(_idx, ts)| ts.first_gid <= gid)
 }
+
+pub fn floor_div(a: i32, b: i32) -> i32 {
+    let d = a / b;
+    let r = a % b;
+
+    if r == 0 {
+        d
+    } else {
+        d - ((a < 0) ^ (b < 0)) as i32
+    }
+}
diff --git a/tests/lib.rs b/tests/lib.rs
index c43bf8c..37e6b7f 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -1,7 +1,7 @@
 use std::path::PathBuf;
 use tiled::{
-    Color, FilesystemResourceCache, FiniteTileLayerData, Layer, LayerDataType, LayerType, Map,
-    ObjectLayer, PropertyValue, ResourceCache, TileLayer, TileLayerData, GroupLayer,
+    Color, FilesystemResourceCache, FiniteTileLayer, GroupLayer, Layer, LayerType, Map,
+    ObjectLayer, PropertyValue, ResourceCache, TileLayer,
 };
 
 fn as_tile_layer<'map>(layer: Layer<'map>) -> TileLayer<'map> {
@@ -11,10 +11,10 @@ fn as_tile_layer<'map>(layer: Layer<'map>) -> TileLayer<'map> {
     }
 }
 
-fn as_finite(data: &TileLayerData) -> &FiniteTileLayerData {
+fn as_finite<'map>(data: TileLayer<'map>) -> FiniteTileLayer<'map> {
     match data {
-        TileLayerData::Finite(data) => data,
-        TileLayerData::Infinite(_) => panic!("Not a finite tile layer"),
+        TileLayer::Finite(data) => data,
+        TileLayer::Infinite(_) => panic!("Not a finite tile layer"),
     }
 }
 
@@ -60,9 +60,9 @@ fn test_gzip_and_zlib_encoded_and_raw_are_the_same() {
     compare_everything_but_tileset_sources(&z, &c);
     compare_everything_but_tileset_sources(&z, &zstd);
 
-    let layer = as_tile_layer(c.get_layer(0).unwrap());
+    let layer = as_finite(as_tile_layer(c.get_layer(0).unwrap()));
     {
-        let data = as_finite(layer.data());
+        let data = layer.data();
         assert_eq!(data.width(), 100);
         assert_eq!(data.height(), 100);
     }
@@ -115,15 +115,33 @@ fn test_infinite_tileset() {
 
     let r = Map::parse_file("assets/tiled_base64_zlib_infinite.tmx", &mut cache).unwrap();
 
-    if let TileLayerData::Infinite(inf) = &as_tile_layer(r.get_layer(0).unwrap()).data() {
-        let chunks = &inf.chunks;
-        assert_eq!(chunks.len(), 4);
-
-        assert_eq!(chunks[&(0, 0)].width, 32);
-        assert_eq!(chunks[&(0, 0)].height, 32);
-        assert_eq!(chunks[&(-32, 0)].width, 32);
-        assert_eq!(chunks[&(0, 32)].height, 32);
-        assert_eq!(chunks[&(-32, 32)].height, 32);
+    if let TileLayer::Infinite(inf) = &as_tile_layer(r.get_layer(1).unwrap()) {
+        assert_eq!(inf.get_tile(2, 10).unwrap().id, 5);
+        assert_eq!(inf.get_tile(5, 36).unwrap().id, 73);
+        assert_eq!(inf.get_tile(15, 15).unwrap().id, 22);
+    } else {
+        assert!(false, "It is wrongly recognised as a finite map");
+    }
+    if let TileLayer::Infinite(inf) = &as_tile_layer(r.get_layer(0).unwrap()) {
+        // NW corner
+        assert_eq!(inf.get_tile(-16, 0).unwrap().id, 17);
+        assert!(inf.get_tile(-17, 0).is_none());
+        assert!(inf.get_tile(-16, -1).is_none());
+
+        // SW corner
+        assert_eq!(inf.get_tile(-16, 47).unwrap().id, 17);
+        assert!(inf.get_tile(-17, 47).is_none());
+        assert!(inf.get_tile(-16, 48).is_none());
+
+        // NE corner
+        assert_eq!(inf.get_tile(31, 0).unwrap().id, 17);
+        assert!(inf.get_tile(31, -1).is_none());
+        assert!(inf.get_tile(32, 0).is_none());
+
+        // SE corner
+        assert_eq!(inf.get_tile(31, 47).unwrap().id, 17);
+        assert!(inf.get_tile(32, 47).is_none());
+        assert!(inf.get_tile(31, 48).is_none());
     } else {
         assert!(false, "It is wrongly recognised as a finite map");
     }
@@ -135,9 +153,9 @@ fn test_image_layers() {
 
     let r = Map::parse_file("assets/tiled_image_layers.tmx", &mut cache).unwrap();
     assert_eq!(r.layers().len(), 2);
-    let mut image_layers = r.layers().map(|layer| layer.data()).map(|layer| {
-        if let LayerDataType::ImageLayer(img) = &layer.layer_type {
-            (img, layer)
+    let mut image_layers = r.layers().map(|layer| {
+        if let LayerType::ImageLayer(img) = &layer.layer_type() {
+            (img.data(), layer.data())
         } else {
             panic!("Found layer that isn't an image layer")
         }
@@ -206,10 +224,8 @@ fn test_object_group_property() {
     let group_layer = r.get_layer(1).unwrap();
     let group_layer = as_group_layer(group_layer);
     let sub_layer = group_layer.get_layer(0).unwrap();
-    let prop_value: bool = if let Some(&PropertyValue::BoolValue(ref v)) = sub_layer
-        .data()
-        .properties
-        .get("an object group property")
+    let prop_value: bool = if let Some(&PropertyValue::BoolValue(ref v)) =
+        sub_layer.data().properties.get("an object group property")
     {
         *v
     } else {
@@ -265,9 +281,9 @@ fn test_ldk_export() {
     let mut cache = FilesystemResourceCache::new();
 
     let r = Map::parse_file("assets/ldk_tiled_export.tmx", &mut cache).unwrap();
-    let layer = as_tile_layer(r.get_layer(0).unwrap());
+    let layer = as_finite(as_tile_layer(r.get_layer(0).unwrap()));
     {
-        let data = as_finite(layer.data());
+        let data = layer.data();
         assert_eq!(data.width(), 8);
         assert_eq!(data.height(), 8);
     }
-- 
GitLab