render: fix shadow map atlas packing by writing my own skyline packer

.vscode/launch.json

@@ -22,6 +22,24 @@
             "args": [],
             "cwd": "${workspaceFolder}/examples/testbed"
         },
+        {
+            "type": "lldb",
+            "request": "launch",
+            "name": "Debug lyra shadows",
+            "cargo": {
+                "args": [
+                    "build",
+                    "--manifest-path", "${workspaceFolder}/examples/shadows/Cargo.toml"
+                    //"--bin=shadows",
+                ],
+                "filter": {
+                    "name": "shadows",
+                    "kind": "bin"
+                }
+            },
+            "args": [],
+            "cwd": "${workspaceFolder}/examples/shadows"
+        },
         {
             "type": "lldb",
             "request": "launch",

Cargo.lock

@@ -1881,7 +1881,6 @@ dependencies = [
  "lyra-scene",
  "petgraph",
  "quote",
- "rectangle-pack",
  "round_mult",
  "rustc-hash",
  "syn 2.0.51",
@@ -2769,12 +2768,6 @@ dependencies = [
  "rand_core 0.3.1",
 ]
 
-[[package]]
-name = "rectangle-pack"
-version = "0.4.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a0d463f2884048e7153449a55166f91028d5b0ea53c79377099ce4e8cf0cf9bb"
-
 [[package]]
 name = "redox_syscall"
 version = "0.3.5"

Cargo.toml

@@ -34,4 +34,4 @@ lyra-scripting = { path = "lyra-scripting", optional = true }
 #opt-level = 1
 
 [profile.release]
-debug = true
+debug = true

examples/shadows/src/main.rs

@@ -6,7 +6,7 @@ use lyra_engine::{
         InputActionPlugin, KeyCode, LayoutId, MouseAxis, MouseInput,
     },
     math::{self, Transform, Vec3},
-    render::light::directional::DirectionalLight,
+    render::light::{directional::DirectionalLight, PointLight},
     scene::{
         CameraComponent, FreeFlyCamera, FreeFlyCameraPlugin, WorldTransform,
         ACTLBL_LOOK_LEFT_RIGHT, ACTLBL_LOOK_ROLL, ACTLBL_LOOK_UP_DOWN,
@@ -130,12 +130,14 @@ fn setup_scene_plugin(game: &mut Game) {
 
     drop(resman);
 
+    // cube in the air
     world.spawn((
         cube_mesh.clone(),
         WorldTransform::default(),
         Transform::from_xyz(0.0, -2.0, -5.0),
     ));
 
+    // cube on the right, on the ground
     world.spawn((
         cube_mesh.clone(),
         WorldTransform::default(),
@@ -163,6 +165,18 @@ fn setup_scene_plugin(game: &mut Game) {
             },
             light_tran,
         ));
+
+        world.spawn((
+            cube_mesh.clone(),
+            PointLight {
+                enabled: true,
+                color: Vec3::new(0.133, 0.098, 0.91),
+                intensity: 2.0,
+                range: 9.0,
+                ..Default::default()
+            },
+            Transform::from_xyz(5.0, -2.5, -3.3),
+        ));
     }
 
     let mut camera = CameraComponent::new_3d();

lyra-game/Cargo.toml

@@ -38,7 +38,6 @@ unique = "0.9.1"
 rustc-hash = "1.1.0"
 petgraph = { version = "0.6.5", features = ["matrix_graph"] }
 bind_match = "0.1.2"
-rectangle-pack = "0.4.2"
 round_mult = "0.1.3"
 
 [features]

lyra-game/src/render/graph/passes/shadows.rs

@@ -11,18 +11,18 @@ use lyra_ecs::{
     AtomicRef, Component, Entity, ResourceData,
 };
 use lyra_game_derive::RenderGraphLabel;
-use lyra_math::Transform;
+use lyra_math::{Angle, Transform};
 use rustc_hash::FxHashMap;
 use tracing::{debug, warn};
 use wgpu::util::DeviceExt;
 
 use crate::render::{
     graph::{Node, NodeDesc, NodeType, SlotAttribute, SlotValue},
-    light::directional::DirectionalLight,
+    light::{directional::DirectionalLight, LightType, PointLight},
     resource::{RenderPipeline, RenderPipelineDescriptor, Shader, VertexState},
     transform_buffer_storage::TransformBuffers,
     vertex::Vertex,
-    AtlasViewport, GpuSlotBuffer, TextureAtlas,
+    AtlasFrame, GpuSlotBuffer, TextureAtlas,
 };
 
 use super::{MeshBufferStorage, RenderAssets, RenderMeshes};
@@ -43,10 +43,15 @@ pub struct ShadowMapsPassLabel;
 
 #[derive(Clone, Copy)]
 struct LightDepthMap {
-    //light_projection_buffer: Arc<wgpu::Buffer>,
+    /// The type of the light that this map is created for.
-    //bindgroup: wgpu::BindGroup,
+    light_type: LightType,
+    /// The index of the first shadow depth map.
+    ///
+    /// If the light is a point light, this is the index of the FIRST depth map in the atlas with
+    /// the maps of the other sides following the index.
     atlas_index: u64,
-    uniform_index: u64,
+    /// The index of the uniform for the light in the uniform array.
+    uniform_index: [u64; 6],
 }
 
 pub struct ShadowMapsPass {
@@ -157,6 +162,7 @@ impl ShadowMapsPass {
     fn create_depth_map(
         &mut self,
         queue: &wgpu::Queue,
+        light_type: LightType,
         entity: Entity,
         light_pos: Transform,
     ) -> LightDepthMap {
@@ -164,34 +170,136 @@ impl ShadowMapsPass {
         const FAR_PLANE: f32 = 45.0;
 
         let mut atlas = self.atlas.get_mut();
-        let atlas_index = atlas
-            .pack_new_texture(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
-            .expect("failed to pack new shadow map into texture atlas");
-        let atlas_frame = atlas.texture_viewport(atlas_index);
 
-        let ortho_proj =
+        let (start_atlas_idx, uniform_indices) = match light_type {
-            glam::Mat4::orthographic_rh(-10.0, 10.0, -10.0, 10.0, NEAR_PLANE, FAR_PLANE);
+            LightType::Directional => {
+                // directional lights require a single map, so allocate that in the atlas.
+                let atlas_index = atlas
+                    .pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .expect("failed to pack new shadow map into texture atlas");
+                let atlas_frame = atlas.texture_frame(atlas_index)
+                    .expect("Frame missing");
 
-        let look_view =
+                let projection =
-            glam::Mat4::look_to_rh(light_pos.translation, light_pos.forward(), light_pos.up());
+                    glam::Mat4::orthographic_rh(-10.0, 10.0, -10.0, 10.0, NEAR_PLANE, FAR_PLANE);
+                let look_view = glam::Mat4::look_to_rh(
+                    light_pos.translation,
+                    light_pos.forward(),
+                    light_pos.up(),
+                );
 
-        let light_proj = ortho_proj * look_view;
+                let light_proj = projection * look_view;
-        let uniform = LightShadowUniform {
+
-            space_mat: light_proj,
+                let u = LightShadowUniform {
-            atlas_frame,
+                    space_mat: light_proj,
+                    atlas_frame,
+                };
+
+                let uniform_index = self.light_uniforms_buffer.insert(queue, &u);
+                let mut indices = [0; 6];
+                indices[0] = uniform_index;
+                (atlas_index, indices)
+            }
+            LightType::Spotlight => todo!(),
+            LightType::Point => {
+                let aspect = SHADOW_SIZE.x as f32 / SHADOW_SIZE.y as f32;
+                let projection = glam::Mat4::perspective_rh(
+                    Angle::Degrees(90.0).to_radians(),
+                    aspect,
+                    NEAR_PLANE,
+                    FAR_PLANE,
+                );
+
+                let light_trans = light_pos.translation;
+                let views = [
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(1.0, 0.0, 0.0),
+                            glam::vec3(0.0, -1.0, 0.0),
+                        ),
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(-1.0, 0.0, 0.0),
+                            glam::vec3(0.0, -1.0, 0.0),
+                        ),
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(0.0, 1.0, 0.0),
+                            glam::vec3(0.0, 0.0, 1.0),
+                        ),
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(0.0, -1.0, 0.0),
+                            glam::vec3(0.0, 0.0, -1.0),
+                        ),
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(0.0, 0.0, 1.0),
+                            glam::vec3(0.0, -1.0, 0.0),
+                        ),
+                    projection
+                        * glam::Mat4::look_at_rh(
+                            light_trans,
+                            light_trans + glam::vec3(0.0, 0.0, -1.0),
+                            glam::vec3(0.0, -1.0, 0.0),
+                        ),
+                ];
+
+                let atlas_idx_1 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+                let atlas_idx_2 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+                let atlas_idx_3 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+                let atlas_idx_4 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+                let atlas_idx_5 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+                let atlas_idx_6 =
+                    atlas.pack(SHADOW_SIZE.x as _, SHADOW_SIZE.y as _)
+                    .unwrap();
+
+                let frames = [
+                    atlas.texture_frame(atlas_idx_1).unwrap(),
+                    atlas.texture_frame(atlas_idx_2).unwrap(),
+                    atlas.texture_frame(atlas_idx_3).unwrap(),
+                    atlas.texture_frame(atlas_idx_4).unwrap(),
+                    atlas.texture_frame(atlas_idx_5).unwrap(),
+                    atlas.texture_frame(atlas_idx_6).unwrap(),
+                ];
+
+                // create the uniforms of the light, storing them in the gpu buffer, and
+                // collecting the indices in the buffer they're at.
+                let mut indices = [0; 6];
+                for i in 0..6 {
+                    let uniform_i = self.light_uniforms_buffer.insert(
+                        queue,
+                        &LightShadowUniform {
+                            space_mat: views[i],
+                            atlas_frame: frames[i],
+                        },
+                    );
+                    indices[i] = uniform_i;
+                }
+
+                (atlas_idx_1, indices)
+            }
         };
 
-        /* let uniform_index = self.light_uniforms_index;
-        self.light_uniforms_index += 1;
-
-        //self.light_uniforms_buffer
-        let offset = uniform_index_offset(&device.limits(), uniform_index);
-        queue.write_buffer(&self.light_uniforms_buffer, offset as u64, bytemuck::bytes_of(&uniform)); */
-        let uniform_index = self.light_uniforms_buffer.insert(queue, &uniform);
-
         let v = LightDepthMap {
-            atlas_index,
+            light_type,
-            uniform_index,
+            atlas_index: start_atlas_idx,
+            uniform_index: uniform_indices,
         };
         self.depth_maps.insert(entity, v);
 
@@ -270,14 +378,40 @@ impl Node for ShadowMapsPass {
         // use a queue for storing atlas ids to add to entities after the entities are iterated
         let mut index_components_queue = VecDeque::new();
 
+        /* for (entity, pos, (has_dir, has_point)) in world.view_iter::<(Entities, &Transform, Or<Has<DirectionalLight>, Has<PointLight>>)>() {
+            if !self.depth_maps.contains_key(&entity) {
+                let light_type = if has_dir.is_some() {
+                    LightType::Directional
+                } else if has_point.is_some() {
+                    LightType::Point
+                } else {
+                    todo!("Spot lights")
+                };
+
+                debug!("Creating depth map for {light_type:?}");
+
+                // TODO: dont pack the textures as they're added
+                let atlas_index =
+                    self.create_depth_map(&context.queue, light_type, entity, *pos);
+                index_components_queue.push_back((entity, atlas_index));
+            }
+        } */
+
         for (entity, pos, _) in world.view_iter::<(Entities, &Transform, Has<DirectionalLight>)>() {
             if !self.depth_maps.contains_key(&entity) {
                 // TODO: dont pack the textures as they're added
                 let atlas_index =
-                    self.create_depth_map(&context.queue, entity, *pos);
+                    self.create_depth_map(&context.queue, LightType::Directional, entity, *pos);
                 index_components_queue.push_back((entity, atlas_index));
+            }
+        }
 
-                debug!("Created depth map for {:?} light entity", entity);
+        for (entity, pos, _) in world.view_iter::<(Entities, &Transform, Has<PointLight>)>() {
+            if !self.depth_maps.contains_key(&entity) {
+                // TODO: dont pack the textures as they're added
+                let atlas_index =
+                    self.create_depth_map(&context.queue, LightType::Point, entity, *pos);
+                index_components_queue.push_back((entity, atlas_index));
             }
         }
 
@@ -287,7 +421,7 @@ impl Node for ShadowMapsPass {
                 entity,
                 LightShadowMapId {
                     atlas_index: depth.atlas_index,
-                    uniform_index: depth.uniform_index,
+                    uniform_indices: depth.uniform_index,
                 },
             );
         }
@@ -332,7 +466,6 @@ impl Node for ShadowMapsPass {
                     multiview: None,
                 },
             ));
-            /*  */
         }
     }
 
@@ -349,102 +482,137 @@ impl Node for ShadowMapsPass {
         let mesh_buffers = self.mesh_buffers();
         let transforms = self.transform_buffers();
 
-        debug_assert_eq!(
+        let atlas = self.atlas.get();
-            self.depth_maps.len(),
+        let mut pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
-            1,
+            label: Some("pass_shadow_map"),
-            "shadows map pass only supports 1 light"
+            color_attachments: &[],
-        );
+            depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
-        let (_, dir_depth_map) = self
+                view: atlas.view(),
-            .depth_maps
+                depth_ops: Some(wgpu::Operations {
-            .iter()
+                    load: wgpu::LoadOp::Clear(1.0),
-            .next()
+                    store: true,
-            .expect("missing directional light in scene");
-
-        {
-            let atlas = self.atlas.get();
-            let mut pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
-                label: Some("pass_shadow_map"),
-                color_attachments: &[],
-                depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
-                    view: atlas.view(),
-                    depth_ops: Some(wgpu::Operations {
-                        load: wgpu::LoadOp::Clear(1.0),
-                        store: true,
-                    }),
-                    stencil_ops: None,
                 }),
-            });
+                stencil_ops: None,
-            pass.set_pipeline(&pipeline);
+            }),
-            let viewport = atlas.texture_viewport(dir_depth_map.atlas_index);
+        });
-            debug!(
+        pass.set_pipeline(&pipeline);
-                "Rendering shadow map to viewport: {viewport:?}, uniform index: {}",
-                dir_depth_map.uniform_index
-            );
-            // only render to the light's map in the atlas
-            pass.set_viewport(
-                viewport.offset.x as _,
-                viewport.offset.y as _,
-                viewport.size.x as _,
-                viewport.size.y as _,
-                0.0,
-                1.0,
-            );
-            // only clear the light map in the atlas
-            pass.set_scissor_rect(
-                viewport.offset.x,
-                viewport.offset.y,
-                viewport.size.x,
-                viewport.size.y,
-            );
 
-            for job in render_meshes.iter() {
+        for light_depth_map in self.depth_maps.values() {
-                // get the mesh (containing vertices) and the buffers from storage
-                let buffers = mesh_buffers.get(&job.mesh_uuid);
-                if buffers.is_none() {
-                    warn!("Skipping job since its mesh is missing {:?}", job.mesh_uuid);
-                    continue;
-                }
-                let buffers = buffers.unwrap();
 
-                let uniform_index =
+            match light_depth_map.light_type {
-                    self.light_uniforms_buffer
+                LightType::Directional => {
-                        .offset_of(dir_depth_map.uniform_index) as u32;
+                    let frame = atlas.texture_frame(light_depth_map.atlas_index)
-                pass.set_bind_group(0, &self.uniforms_bg, &[uniform_index]);
+                        .expect("missing atlas frame of light");
+                    let u_offset = self.light_uniforms_buffer.offset_of(light_depth_map.uniform_index[0]) as u32;
 
-                // Get the bindgroup for job's transform and bind to it using an offset.
+                    //debug!("Rendering directional light with atlas {} uniform index {} and offset {}, in viewport {:?}", light_depth_map.atlas_index, light_depth_map.uniform_index[0], u_offset, frame);
-                let bindgroup = transforms.bind_group(job.transform_id);
-                let offset = transforms.buffer_offset(job.transform_id);
-                pass.set_bind_group(1, bindgroup, &[offset]);
 
-                // if this mesh uses indices, use them to draw the mesh
+                    light_shadow_pass_impl(
-                if let Some((idx_type, indices)) = buffers.buffer_indices.as_ref() {
+                        &mut pass,
-                    let indices_len = indices.count() as u32;
+                        &self.uniforms_bg,
-
+                        &render_meshes,
-                    pass.set_vertex_buffer(
+                        &mesh_buffers,
-                        buffers.buffer_vertex.slot(),
+                        &transforms,
-                        buffers.buffer_vertex.buffer().slice(..),
+                        &frame,
+                        u_offset,
                     );
-                    pass.set_index_buffer(indices.buffer().slice(..), *idx_type);
+                },
-                    pass.draw_indexed(0..indices_len, 0, 0..1);
+                LightType::Point => {
-                } else {
+                    for side in 0..6 {
-                    let vertex_count = buffers.buffer_vertex.count();
+                        let frame = atlas.texture_frame(light_depth_map.atlas_index + side)
+                            .expect("missing atlas frame of light");
+                        let ui = light_depth_map.uniform_index[side as usize];
+                        let u_offset = self.light_uniforms_buffer.offset_of(ui) as u32;
+                    
+                        //debug!("Rendering point light side {side} with atlas {} uniform index {ui} and offset {u_offset} and viewport {:?}", light_depth_map.atlas_index + side, frame);
 
-                    pass.set_vertex_buffer(
+                        light_shadow_pass_impl(
-                        buffers.buffer_vertex.slot(),
+                            &mut pass,
-                        buffers.buffer_vertex.buffer().slice(..),
+                            &self.uniforms_bg,
-                    );
+                            &render_meshes,
-                    pass.draw(0..vertex_count as u32, 0..1);
+                            &mesh_buffers,
-                }
+                            &transforms,
+                            &frame,
+                            u_offset,
+                        );
+                    }
+                },
+                LightType::Spotlight => todo!(),
             }
         }
     }
 }
 
+fn light_shadow_pass_impl<'a>(
+    pass: &mut wgpu::RenderPass<'a>,
+    uniforms_bind_group: &'a wgpu::BindGroup,
+    render_meshes: &RenderMeshes,
+    mesh_buffers: &'a RenderAssets<MeshBufferStorage>,
+    transforms: &'a TransformBuffers,
+    shadow_atlas_viewport: &AtlasFrame,
+    uniform_offset: u32,
+) {
+    // only render to the light's map in the atlas
+    pass.set_viewport(
+        shadow_atlas_viewport.x as _,
+        shadow_atlas_viewport.y as _,
+        shadow_atlas_viewport.width as _,
+        shadow_atlas_viewport.height as _,
+        0.0,
+        1.0,
+    );
+    // only clear the light map in the atlas
+    pass.set_scissor_rect(
+        shadow_atlas_viewport.x as _,
+        shadow_atlas_viewport.y as _,
+        shadow_atlas_viewport.width as _,
+        shadow_atlas_viewport.height as _,
+    );
+
+    for job in render_meshes.iter() {
+        // get the mesh (containing vertices) and the buffers from storage
+        let buffers = mesh_buffers.get(&job.mesh_uuid);
+        if buffers.is_none() {
+            warn!("Skipping job since its mesh is missing {:?}", job.mesh_uuid);
+            continue;
+        }
+        let buffers = buffers.unwrap();
+
+        //let uniform_index = light_uniforms_buffer.offset_of(light_depth_map.uniform_index[0]) as u32;
+        pass.set_bind_group(0, &uniforms_bind_group, &[uniform_offset]);
+
+        // Get the bindgroup for job's transform and bind to it using an offset.
+        let bindgroup = transforms.bind_group(job.transform_id);
+        let offset = transforms.buffer_offset(job.transform_id);
+        pass.set_bind_group(1, bindgroup, &[offset]);
+
+        // if this mesh uses indices, use them to draw the mesh
+        if let Some((idx_type, indices)) = buffers.buffer_indices.as_ref() {
+            let indices_len = indices.count() as u32;
+
+            pass.set_vertex_buffer(
+                buffers.buffer_vertex.slot(),
+                buffers.buffer_vertex.buffer().slice(..),
+            );
+            pass.set_index_buffer(indices.buffer().slice(..), *idx_type);
+            pass.draw_indexed(0..indices_len, 0, 0..1);
+        } else {
+            let vertex_count = buffers.buffer_vertex.count();
+
+            pass.set_vertex_buffer(
+                buffers.buffer_vertex.slot(),
+                buffers.buffer_vertex.buffer().slice(..),
+            );
+            pass.draw(0..vertex_count as u32, 0..1);
+        }
+    }
+}
+
 #[repr(C)]
 #[derive(Debug, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
 pub struct LightShadowUniform {
     space_mat: glam::Mat4,
-    atlas_frame: AtlasViewport, // 2xUVec2 (4xf32), so no padding needed
+    atlas_frame: AtlasFrame, // 2xUVec2 (4xf32), so no padding needed
 }
 
 /// A component that stores the ID of a shadow map in the shadow map atlas for the entities.
@@ -454,7 +622,7 @@ pub struct LightShadowUniform {
 #[derive(Debug, Default, Copy, Clone, Component)]
 pub struct LightShadowMapId {
     atlas_index: u64,
-    uniform_index: u64,
+    uniform_indices: [u64; 6],
 }
 
 impl LightShadowMapId {
@@ -462,8 +630,8 @@ impl LightShadowMapId {
         self.atlas_index
     }
 
-    pub fn uniform_index(&self) -> u64 {
+    pub fn uniform_index(&self, side: usize) -> u64 {
-        self.uniform_index
+        self.uniform_indices[side]
     }
 }
 

lyra-game/src/render/light/mod.rs

@@ -1,12 +1,17 @@
-pub mod point;
 pub mod directional;
+pub mod point;
 pub mod spotlight;
 
 use lyra_ecs::{Entity, Tick, World};
 pub use point::*;
 pub use spotlight::*;
 
-use std::{collections::{HashMap, VecDeque}, marker::PhantomData, mem, sync::Arc};
+use std::{
+    collections::{HashMap, VecDeque},
+    marker::PhantomData,
+    mem,
+    sync::Arc,
+};
 
 use crate::math::Transform;
 
@@ -22,7 +27,7 @@ pub struct LightBuffer<U: Default + bytemuck::Pod + bytemuck::Zeroable> {
     /// The max amount of light casters that could fit in this buffer.
     pub max_count: usize,
     /// The amount of light casters that are taking up space in the buffer.
-    /// 
+    ///
     /// This means that a light may be inactive in the buffer, by being replaced
     /// with a default caster as to not affect lighting. Its easier this way than
     /// to recreate the array and remove the gaps.
@@ -49,15 +54,27 @@ impl<U: Default + bytemuck::Pod + bytemuck::Zeroable> LightBuffer<U> {
     }
 
     /// Update an existing light in the light buffer.
-    pub fn update_light(&mut self, lights_buffer: &mut [U; MAX_LIGHT_COUNT], entity: Entity, light: U) {
+    pub fn update_light(
-        let buffer_idx = *self.used_indexes.get(&entity)
+        &mut self,
+        lights_buffer: &mut [U; MAX_LIGHT_COUNT],
+        entity: Entity,
+        light: U,
+    ) {
+        let buffer_idx = *self
+            .used_indexes
+            .get(&entity)
             .expect("Entity for Light is not in buffer!");
 
         lights_buffer[buffer_idx] = light;
     }
 
     /// Add a new light to the light buffer.
-    pub fn add_light(&mut self, lights_buffer: &mut [U; MAX_LIGHT_COUNT], entity: Entity, light: U) {
+    pub fn add_light(
+        &mut self,
+        lights_buffer: &mut [U; MAX_LIGHT_COUNT],
+        entity: Entity,
+        light: U,
+    ) {
         let buffer_idx = match self.dead_indexes.pop_front() {
             Some(i) => i,
             None => {
@@ -69,15 +86,20 @@ impl<U: Default + bytemuck::Pod + bytemuck::Zeroable> LightBuffer<U> {
                 assert!(self.buffer_count <= self.max_count);
 
                 i
-            },
+            }
         };
-        
+
         self.used_indexes.insert(entity, buffer_idx);
         self.update_light(lights_buffer, entity, light);
     }
 
     /// Update, or add a new caster, to the light buffer.
-    pub fn update_or_add(&mut self, lights_buffer: &mut [U; MAX_LIGHT_COUNT], entity: Entity, light: U) {
+    pub fn update_or_add(
+        &mut self,
+        lights_buffer: &mut [U; MAX_LIGHT_COUNT],
+        entity: Entity,
+        light: U,
+    ) {
         if self.used_indexes.contains_key(&entity) {
             self.update_light(lights_buffer, entity, light);
         } else {
@@ -86,7 +108,11 @@ impl<U: Default + bytemuck::Pod + bytemuck::Zeroable> LightBuffer<U> {
     }
 
     /// Remove a caster from the buffer, returns true if it was removed.
-    pub fn remove_light(&mut self, lights_buffer: &mut [U; MAX_LIGHT_COUNT], entity: Entity) -> bool {
+    pub fn remove_light(
+        &mut self,
+        lights_buffer: &mut [U; MAX_LIGHT_COUNT],
+        entity: Entity,
+    ) -> bool {
         if let Some(removed_idx) = self.used_indexes.remove(&entity) {
             self.dead_indexes.push_back(removed_idx);
             //self.current_count -= 1;
@@ -112,47 +138,37 @@ impl LightUniformBuffers {
         // TODO: ensure we dont write over this limit
         let max_buffer_sizes = (limits.max_uniform_buffer_binding_size as u64) / 2;
 
-        let buffer = device.create_buffer(
+        let buffer = device.create_buffer(&wgpu::BufferDescriptor {
-            &wgpu::BufferDescriptor {
+            label: Some("UBO_Lights"),
-                label: Some("UBO_Lights"),
+            usage: wgpu::BufferUsages::STORAGE | wgpu::BufferUsages::COPY_DST,
-                usage: wgpu::BufferUsages::STORAGE | wgpu::BufferUsages::COPY_DST,
+            size: max_buffer_sizes,
-                size: max_buffer_sizes,
+            mapped_at_creation: false,
-                mapped_at_creation: false,
+        });
-            }
-        );
 
         let bindgroup_layout = device.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
-            entries: &[
+            entries: &[wgpu::BindGroupLayoutEntry {
-                wgpu::BindGroupLayoutEntry {
+                binding: 0,
-                    binding: 0,
+                visibility: wgpu::ShaderStages::FRAGMENT | wgpu::ShaderStages::COMPUTE,
-                    visibility: wgpu::ShaderStages::FRAGMENT | wgpu::ShaderStages::COMPUTE,
+                ty: wgpu::BindingType::Buffer {
-                    ty: wgpu::BindingType::Buffer {
+                    ty: wgpu::BufferBindingType::Storage { read_only: true },
-                        ty: wgpu::BufferBindingType::Storage {
+                    has_dynamic_offset: false,
-                            read_only: true
+                    min_binding_size: None,
-                        },
-                        has_dynamic_offset: false,
-                        min_binding_size: None,
-                    },
-                    count: None,
                 },
-            ],
+                count: None,
+            }],
             label: Some("BGL_Lights"),
         });
 
         let bindgroup = device.create_bind_group(&wgpu::BindGroupDescriptor {
             layout: &bindgroup_layout,
-            entries: &[
+            entries: &[wgpu::BindGroupEntry {
-                wgpu::BindGroupEntry {
+                binding: 0,
-                    binding: 0,
+                resource: wgpu::BindingResource::Buffer(wgpu::BufferBinding {
-                    resource: wgpu::BindingResource::Buffer(
+                    buffer: &buffer,
-                            wgpu::BufferBinding {
+                    offset: 0,
-                            buffer: &buffer,
+                    size: None, // use the full buffer
-                            offset: 0,
+                }),
-                            size: None, // use the full buffer
+            }],
-                        }
-                    )
-                },
-            ],
             label: Some("BG_Lights"),
         });
 
@@ -168,21 +184,30 @@ impl LightUniformBuffers {
         let _ = world_tick;
         let mut lights = vec![];
 
-        for (point_light, transform, shadow_map_id) in world.view_iter::<(&PointLight, &Transform, Option<&LightShadowMapId>)>() {
+        for (point_light, transform, shadow_map_id) in
+            world.view_iter::<(&PointLight, &Transform, Option<&LightShadowMapId>)>()
+        {
             let shadow_map_id = shadow_map_id.map(|m| m.clone());
-            let uniform = LightUniform::from_point_light_bundle(&point_light, &transform, shadow_map_id);
+            let uniform =
+                LightUniform::from_point_light_bundle(&point_light, &transform, shadow_map_id);
             lights.push(uniform);
         }
 
-        for (spot_light, transform, shadow_map_id) in world.view_iter::<(&SpotLight, &Transform, Option<&LightShadowMapId>)>() {
+        for (spot_light, transform, shadow_map_id) in
+            world.view_iter::<(&SpotLight, &Transform, Option<&LightShadowMapId>)>()
+        {
             let shadow_map_id = shadow_map_id.map(|m| m.clone());
-            let uniform = LightUniform::from_spot_light_bundle(&spot_light, &transform, shadow_map_id);
+            let uniform =
+                LightUniform::from_spot_light_bundle(&spot_light, &transform, shadow_map_id);
             lights.push(uniform);
         }
 
-        for (dir_light, transform, shadow_map_id) in world.view_iter::<(&DirectionalLight, &Transform, Option<&LightShadowMapId>)>() {
+        for (dir_light, transform, shadow_map_id) in
+            world.view_iter::<(&DirectionalLight, &Transform, Option<&LightShadowMapId>)>()
+        {
             let shadow_map_id = shadow_map_id.map(|m| m.clone());
-            let uniform = LightUniform::from_directional_bundle(&dir_light, &transform, shadow_map_id);
+            let uniform =
+                LightUniform::from_directional_bundle(&dir_light, &transform, shadow_map_id);
             lights.push(uniform);
         }
 
@@ -191,7 +216,11 @@ impl LightUniformBuffers {
         // write the amount of lights to the buffer, and right after that the list of lights.
         queue.write_buffer(&self.buffer, 0, bytemuck::cast_slice(&[lights.len()]));
         // the size of u32 is multiplied by 4 because of gpu alignment requirements
-        queue.write_buffer(&self.buffer, mem::size_of::<u32>() as u64 * 4, bytemuck::cast_slice(lights.as_slice()));
+        queue.write_buffer(
+            &self.buffer,
+            mem::size_of::<u32>() as u64 * 4,
+            bytemuck::cast_slice(lights.as_slice()),
+        );
     }
 }
 
@@ -214,18 +243,22 @@ pub(crate) struct LightUniform {
     pub color: glam::Vec3,
     // no padding is needed here since range acts as the padding
     // that would usually be needed for the vec3
-
     pub range: f32,
     pub intensity: f32,
     pub smoothness: f32,
 
     pub spot_cutoff_rad: f32,
     pub spot_outer_cutoff_rad: f32,
-    pub light_shadow_uniform_index: i32,
+    pub light_shadow_uniform_index: [i32; 6],
+    _padding: [u32; 2],
 }
 
 impl LightUniform {
-    pub fn from_point_light_bundle(light: &PointLight, transform: &Transform, map_id: Option<LightShadowMapId>) -> Self {
+    pub fn from_point_light_bundle(
+        light: &PointLight,
+        transform: &Transform,
+        map_id: Option<LightShadowMapId>,
+    ) -> Self {
         Self {
             light_type: LightType::Point as u32,
             enabled: light.enabled as u32,
@@ -239,12 +272,27 @@ impl LightUniform {
 
             spot_cutoff_rad: 0.0,
             spot_outer_cutoff_rad: 0.0,
-            light_shadow_uniform_index:  map_id.map(|m| m.uniform_index() as i32).unwrap_or(-1),
+            light_shadow_uniform_index: map_id
-
+                .map(|m| {
+                    [
+                        m.uniform_index(0) as i32,
+                        m.uniform_index(1) as i32,
+                        m.uniform_index(2) as i32,
+                        m.uniform_index(3) as i32,
+                        m.uniform_index(4) as i32,
+                        m.uniform_index(5) as i32,
+                    ]
+                })
+                .unwrap_or([-1; 6]),
+            _padding: [0; 2],
         }
     }
 
-    pub fn from_directional_bundle(light: &DirectionalLight, transform: &Transform, map_id: Option<LightShadowMapId>) -> Self {
+    pub fn from_directional_bundle(
+        light: &DirectionalLight,
+        transform: &Transform,
+        map_id: Option<LightShadowMapId>,
+    ) -> Self {
         Self {
             light_type: LightType::Directional as u32,
             enabled: light.enabled as u32,
@@ -258,12 +306,28 @@ impl LightUniform {
 
             spot_cutoff_rad: 0.0,
             spot_outer_cutoff_rad: 0.0,
-            light_shadow_uniform_index:  map_id.map(|m| m.uniform_index() as i32).unwrap_or(-1),
+            light_shadow_uniform_index: map_id
+                .map(|m| {
+                    [
+                        m.uniform_index(0) as i32,
+                        m.uniform_index(1) as i32,
+                        m.uniform_index(2) as i32,
+                        m.uniform_index(3) as i32,
+                        m.uniform_index(4) as i32,
+                        m.uniform_index(5) as i32,
+                    ]
+                })
+                .unwrap_or([-1; 6]),
+            _padding: [0; 2],
         }
     }
 
     // Create the SpotLightUniform from an ECS bundle
-    pub fn from_spot_light_bundle(light: &SpotLight, transform: &Transform, map_id: Option<LightShadowMapId>) -> Self {
+    pub fn from_spot_light_bundle(
+        light: &SpotLight,
+        transform: &Transform,
+        map_id: Option<LightShadowMapId>,
+    ) -> Self {
         Self {
             light_type: LightType::Spotlight as u32,
             enabled: light.enabled as u32,
@@ -277,8 +341,19 @@ impl LightUniform {
 
             spot_cutoff_rad: light.cutoff.to_radians(),
             spot_outer_cutoff_rad: light.outer_cutoff.to_radians(),
-            light_shadow_uniform_index:  map_id.map(|m| m.uniform_index() as i32).unwrap_or(-1),
+            light_shadow_uniform_index: map_id
+                .map(|m| {
+                    [
+                        m.uniform_index(0) as i32,
+                        m.uniform_index(1) as i32,
+                        m.uniform_index(2) as i32,
+                        m.uniform_index(3) as i32,
+                        m.uniform_index(4) as i32,
+                        m.uniform_index(5) as i32,
+                    ]
+                })
+                .unwrap_or([-1; 6]),
+            _padding: [0; 2],
         }
     }
 }
-

lyra-game/src/render/shaders/base.wgsl

@@ -54,7 +54,7 @@ struct Light {
 
     spot_cutoff: f32,
     spot_outer_cutoff: f32,
-    light_shadow_uniform_index: i32,
+    light_shadow_uniform_index: array<i32, 6>,
 };
 
 struct Lights {
@@ -114,7 +114,7 @@ struct LightShadowMapUniform {
 }
 
 struct LightShadowMapUniformAligned {
-    @size(256)
+    @align(256)
     inner: LightShadowMapUniform
 }
 
@@ -160,10 +160,10 @@ fn fs_main(in: VertexOutput) -> @location(0) vec4<f32> {
 
         if (light.light_ty == LIGHT_TY_DIRECTIONAL) {
             let light_dir = normalize(-light.direction);
-            let shadow_u: LightShadowMapUniform = u_light_shadow[light.light_shadow_uniform_index].inner;
+            let shadow_u: LightShadowMapUniform = u_light_shadow[light.light_shadow_uniform_index[0]].inner;
             let frag_pos_light_space = shadow_u.light_space_matrix * vec4<f32>(in.world_position, 1.0);
 
-            let shadow = calc_shadow(in.world_normal, light_dir, frag_pos_light_space, atlas_dimensions, shadow_u.atlas_frame);
+            let shadow = calc_shadow_dir_light(in.world_normal, light_dir, frag_pos_light_space, atlas_dimensions, shadow_u.atlas_frame);
             light_res += blinn_phong_dir_light(in.world_position, in.world_normal, light, u_material, specular_color, shadow);
         } else if (light.light_ty == LIGHT_TY_POINT) {
             light_res += blinn_phong_point_light(in.world_position, in.world_normal, light, u_material, specular_color);
@@ -176,7 +176,7 @@ fn fs_main(in: VertexOutput) -> @location(0) vec4<f32> {
     return vec4<f32>(light_object_res, object_color.a);
 }
 
-fn calc_shadow(normal: vec3<f32>, light_dir: vec3<f32>, frag_pos_light_space: vec4<f32>, atlas_dimensions: vec2<i32>, atlas_region: TextureAtlasFrame) -> f32 {
+fn calc_shadow_dir_light(normal: vec3<f32>, light_dir: vec3<f32>, frag_pos_light_space: vec4<f32>, atlas_dimensions: vec2<i32>, atlas_region: TextureAtlasFrame) -> f32 {
     var proj_coords = frag_pos_light_space.xyz / frag_pos_light_space.w;
     // for some reason the y component is clipped after transforming
     proj_coords.y = -proj_coords.y;
@@ -218,6 +218,10 @@ fn calc_shadow(normal: vec3<f32>, light_dir: vec3<f32>, frag_pos_light_space: ve
     return shadow;
 }
 
+fn calc_shadow_point(world_pos: vec3<f32>, atlas_dimensions: vec2<i32>, atlas_regions: array<TextureAtlasFrame, 6>) -> f32 {
+    return 0.0;
+}
+
 fn debug_grid(in: VertexOutput) -> vec4<f32> {
     let tile_index_float: vec2<f32> = in.clip_position.xy / 16.0;
     let tile_index = vec2<u32>(floor(tile_index_float));

lyra-game/src/render/shaders/light_cull.comp.wgsl

@@ -31,6 +31,7 @@ struct Light {
 
     spot_cutoff: f32,
     spot_outer_cutoff: f32,
+    light_shadow_uniform_index: array<i32, 6>,
 };
 
 struct Lights {

lyra-game/src/render/slot_buffer.rs

@@ -1,4 +1,4 @@
-use std::{collections::VecDeque, marker::PhantomData, mem, num::NonZeroU64, sync::Arc};
+use std::{collections::VecDeque, marker::PhantomData, mem, sync::Arc};
 
 /// A buffer on the GPU that has persistent indices.
 /// 
@@ -54,12 +54,19 @@ impl<T: bytemuck::Pod + bytemuck::Zeroable> GpuSlotBuffer<T> {
 
     /// Calculates the byte offset in the buffer of the element at `i`.
     pub fn offset_of(&self, i: u64) -> u64 {
-        let offset = i * mem::size_of::<T>() as u64;
+        /* let offset = i * mem::size_of::<T>() as u64;
 
         if let Some(align) = self.alignment {
             round_mult::up(offset, NonZeroU64::new(align).unwrap()).unwrap()
         } else {
             offset
+        } */
+
+        if let Some(align) = self.alignment {
+            let transform_index = i % self.capacity;
+            transform_index * align
+        } else {
+            mem::size_of::<T>() as u64
         }
     }
 

lyra-game/src/render/texture_atlas.rs

@@ -1,10 +1,8 @@
 use std::{
-    collections::BTreeMap,
+    cmp::max, collections::HashMap, sync::Arc
-    sync::Arc,
 };
 
 use glam::UVec2;
-use rectangle_pack::{pack_rects, GroupedRectsToPlace, RectToInsert, RectanglePackOk, TargetBin};
 
 #[derive(Debug, thiserror::Error)]
 pub enum AtlasPackError {
@@ -14,28 +12,33 @@ pub enum AtlasPackError {
 }
 
 #[repr(C)]
-#[derive(Debug, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
+#[derive(Debug, Default, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
-pub struct AtlasViewport {
+pub struct AtlasFrame {
-    pub offset: UVec2,
+    pub x: u32,
-    pub size: UVec2,
+    pub y: u32,
+    pub width: u32,
+    pub height: u32,
 }
 
-pub struct TextureAtlas {
+impl AtlasFrame {
+    pub fn new(x: u32, y: u32, width: u32, height: u32) -> Self {
+        Self {
+            x, y, width, height
+        }
+    }
+}
+
+pub struct TextureAtlas<P: AtlasPacker = SkylinePacker> {
     atlas_size: UVec2,
 
     texture_format: wgpu::TextureFormat,
     texture: Arc<wgpu::Texture>,
     view: Arc<wgpu::TextureView>,
 
-    /// The next id of the next texture that will be added to the atlas.
+    packer: P,
-    next_texture_id: u64,
-
-    rects: GroupedRectsToPlace<u64>,
-    bins: BTreeMap<u64, TargetBin>,
-    placement: Option<RectanglePackOk<u64, u64>>,
 }
 
-impl TextureAtlas {
+impl<P: AtlasPacker> TextureAtlas<P> {
     pub fn new(
         device: &wgpu::Device,
         format: wgpu::TextureFormat,
@@ -58,98 +61,30 @@ impl TextureAtlas {
         });
         let view = texture.create_view(&wgpu::TextureViewDescriptor::default());
 
-        let mut bins = BTreeMap::new();
-        // max_depth=1 for 2d
-        bins.insert(0, TargetBin::new(atlas_size.x, atlas_size.y, 1));
-
         Self {
             atlas_size,
             texture_format: format,
             texture: Arc::new(texture),
             view: Arc::new(view),
-            next_texture_id: 0,
+            packer: P::new(atlas_size),
-            rects: GroupedRectsToPlace::new(),
-            bins,
-            placement: None,
         }
     }
 
     /// Add a texture of `size` and pack it into the atlas, returning the id of the texture in
     /// the atlas.
-    /// 
+    ///
     /// If you are adding multiple textures at a time and want to wait to pack the atlas, use
     /// [`TextureAtlas::add_texture_unpacked`] and then after you're done adding them, pack them
     /// with [`TextureAtlas::pack_atlas`].
-    pub fn pack_new_texture(&mut self, width: u32, height: u32) -> Result<u64, AtlasPackError> {
+    pub fn pack(&mut self, width: u32, height: u32) -> Result<u64, AtlasPackError> {
-        let id = self.next_texture_id;
+        let id = self.packer.pack(width, height)?;
-        self.next_texture_id += 1;
 
-        // for 2d rects, set depth to 1
+        Ok(id as u64)
-        let r = RectToInsert::new(width, height, 1);
-        self.rects.push_rect(id, None, r);
-
-        self.pack_atlas()?;
-
-        Ok(id)
-    }
-
-    /// Add a new texture and **DO NOT** pack it into the atlas.
-    /// 
-    /// <div class="warning">
-    /// 
-    /// The texture will not be packed into the atlas meaning
-    /// [`TextureAtlas::texture_viewport`] will return `None`. To pack the texture,
-    /// use [`TextureAtlas::pack_atlas`] or use [`TextureAtlas::pack_new_texture`]
-    /// when only adding a single texture.
-    /// 
-    /// </div>
-    pub fn add_texture_unpacked(&mut self, width: u32, height: u32) -> Result<u64, AtlasPackError> {
-        let id = self.next_texture_id;
-        self.next_texture_id += 1;
-
-        // for 2d rects, set depth to 1
-        let r = RectToInsert::new(width, height, 1);
-        self.rects.push_rect(id, None, r);
-
-        self.pack_atlas()?;
-
-        Ok(id)
-    }
-
-    /// Pack the textures into the atlas.
-    pub fn pack_atlas(&mut self) -> Result<(), AtlasPackError> {
-        let placement = pack_rects(
-            &self.rects,
-            &mut self.bins,
-            &rectangle_pack::volume_heuristic,
-            &rectangle_pack::contains_smallest_box,
-        )
-        .map_err(|e| match e {
-            rectangle_pack::RectanglePackError::NotEnoughBinSpace => AtlasPackError::NotEnoughSpace,
-        })?;
-        self.placement = Some(placement);
-
-        Ok(())
     }
 
     /// Get the viewport of a texture index in the atlas.
-    pub fn texture_viewport(&self, atlas_index: u64) -> AtlasViewport {
+    pub fn texture_frame(&self, atlas_index: u64) -> Option<AtlasFrame> {
-        let locations = self.placement.as_ref().unwrap().packed_locations();
+        self.packer.frame(atlas_index as _)
-        let (bin_id, loc) = locations
-            .get(&atlas_index)
-            .expect("atlas index is incorrect");
-        debug_assert_eq!(*bin_id, 0, "somehow the texture was put in some other bin");
-
-        AtlasViewport {
-            offset: UVec2 {
-                x: loc.x(),
-                y: loc.y(),
-            },
-            size: UVec2 {
-                x: loc.width(),
-                y: loc.height(),
-            },
-        }
     }
 
     pub fn view(&self) -> &Arc<wgpu::TextureView> {
@@ -164,12 +99,199 @@ impl TextureAtlas {
         &self.texture_format
     }
 
-    pub fn total_texture_count(&self) -> u64 {
-        self.next_texture_id // starts at zero, so no need to increment
-    }
-
     /// Returns the size of the entire texture atlas.
     pub fn atlas_size(&self) -> UVec2 {
         self.atlas_size
     }
 }
+
+pub trait AtlasPacker {
+    fn new(size: UVec2) -> Self;
+
+    /// Get an [`AtlasFrame`] of a texture with `id`.
+    fn frame(&self, id: usize) -> Option<AtlasFrame>;
+    
+    /// Get all [`AtlasFrame`]s in the atlas.
+    fn frames(&self) -> &HashMap<usize, AtlasFrame>;
+
+    /// Pack a new rect into the atlas.
+    fn pack(&mut self, width: u32, height: u32) -> Result<usize, AtlasPackError>;
+}
+
+struct Skyline {
+    /// Starting x of the skyline
+    x: usize,
+    /// Starting y of the skyline
+    y: usize,
+    /// Width of the skyline
+    width: usize,
+}
+
+impl Skyline {
+    fn right(&self) -> usize {
+        self.x + self.width
+    }
+}
+
+pub struct SkylinePacker {
+    size: UVec2,
+    skylines: Vec<Skyline>,
+    frame_idx: usize,
+    frames: HashMap<usize, AtlasFrame>,
+}
+
+impl SkylinePacker {
+    pub fn new(size: UVec2) -> Self {
+        let skylines = vec![Skyline {
+            x: 0,
+            y: 0,
+            width: size.x as _,
+        }];
+
+        Self {
+            size,
+            skylines,
+            frame_idx: 0,
+            frames: Default::default(),
+        }
+    }
+
+    fn can_add(&self, mut i: usize, w: u32, h: u32) -> Option<usize> {
+        let x = self.skylines[i].x as u32;
+        if x + w > self.size.x {
+            return None;
+        }
+
+        let mut width_left = w;
+        let mut y = self.skylines[i].y as u32;
+
+        loop {
+            y = max(y, self.skylines[i].y as u32);
+
+            if y + h > self.size.y {
+                return None;
+            }
+
+            if self.skylines[i].width as u32 > width_left {
+                return Some(y as usize);
+            }
+
+            width_left -= self.skylines[i].width as u32;
+            i += 1;
+
+            if i >= self.skylines.len() {
+                return None;
+            }
+        }
+    }
+
+    fn find_skyline(&self, width: u32, height: u32) -> Option<(usize, AtlasFrame)> {
+        let mut min_height = std::u32::MAX;
+        let mut min_width = std::u32::MAX;
+        let mut index = None;
+        let mut frame = AtlasFrame::default();
+
+        // keep the min height as small as possible
+        for i in 0..self.skylines.len() {
+            if let Some(y) = self.can_add(i, width, height) {
+                let y = y as u32;
+                /* if r.bottom() < min_height
+                    || (r.bottom() == min_height && self.skylines[i].width < min_width as usize) */
+                if y + height < min_height ||
+                    (y + height == min_height && self.skylines[i].width < min_width as _)
+                {
+                    min_height = y + height;
+                    min_width = self.skylines[i].width as _;
+                    index = Some(i);
+                    frame = AtlasFrame::new(self.skylines[i].x as _, y, width, height);
+                }
+            }
+
+            // TODO: rotation
+        }
+
+        if let Some(index) = index {
+            Some((index, frame))
+        } else {
+            None
+        }
+    }
+
+    fn split(&mut self, i: usize, frame: &AtlasFrame) {
+        let skyline = Skyline {
+            x: frame.x as _,
+            y: (frame.y + frame.height) as _,
+            width: frame.width as _
+        };
+
+        assert!(skyline.right() <= self.size.x as _);
+        assert!(skyline.y <= self.size.y as _);
+
+        self.skylines.insert(i, skyline);
+
+        let i = i + 1;
+
+        while i < self.skylines.len() {
+            assert!(self.skylines[i - 1].x <= self.skylines[i].x);
+
+            if self.skylines[i].x < self.skylines[i - 1].x + self.skylines[i - 1].width {
+                let shrink = self.skylines[i-1].x + self.skylines[i-1].width - self.skylines[i].x;
+
+                if self.skylines[i].width <= shrink {
+                    self.skylines.remove(i);
+                } else {
+                    self.skylines[i].x += shrink;
+                    self.skylines[i].width -= shrink;
+                    break;
+                }
+            } else {
+                break;
+            }
+        }
+    }
+
+    /// Merge skylines with the same y value
+    fn merge(&mut self) {
+        let mut i = 1;
+        while i < self.skylines.len() {
+            if self.skylines[i - 1].y == self.skylines[i].y {
+                self.skylines[i - 1].width += self.skylines[i].width;
+                self.skylines.remove(i);
+            } else {
+                i += 1;
+            }
+        }
+    }
+
+    //pub fn pack(&mut self, )
+}
+
+impl AtlasPacker for SkylinePacker {
+    fn new(size: UVec2) -> Self {
+        SkylinePacker::new(size)
+    }
+
+    fn frame(&self, id: usize) -> Option<AtlasFrame> {
+        self.frames.get(&id).cloned()
+    }
+
+    fn frames(&self) -> &HashMap<usize, AtlasFrame> {
+        &self.frames
+    }
+
+    fn pack(&mut self, width: u32, height: u32) -> Result<usize, AtlasPackError> {
+        if let Some((i, frame)) = self.find_skyline(width, height) {
+            self.split(i, &frame);
+            self.merge();
+
+            let frame_idx = self.frame_idx;
+            self.frame_idx += 1;
+
+            self.frames.insert(frame_idx, frame);
+
+            Ok(frame_idx)
+        } else {
+            Err(AtlasPackError::NotEnoughSpace)
+        }
+    }
+}