Address various TODOs introduced in wgpu transition

assets/voxygen/shaders/skybox-vert.glsl

@@ -25,8 +25,16 @@ layout(location = 0) out vec3 f_pos;
 void main() {
     f_pos = v_pos;
 
-    // TODO: is this todo below still valid? is cam_pos jittery
     // TODO: Make this position-independent to avoid rounding error jittering
+    // NOTE: we may or may not want to use an infinite projection here
+    //
+    // Essentially: using any finite projection is likely wrong here if we want
+    // to project out to infinity, but since we want to perturb the skybox as we
+    // move and we have stars now, the "right" answer is heavily dependent on
+    // how we compute cloud position and stuff.
+    //
+    // Infinite projections of cubemaps are nice because they can be oriented
+    // but still extend infinitely far.
     gl_Position =
         all_mat *
         vec4(v_pos + cam_pos.xyz, 1);

assets/voxygen/shaders/terrain-vert.glsl

@@ -32,6 +32,7 @@ layout (std140, set = 2, binding = 0)
 uniform u_locals {
     vec3 model_offs;
     float load_time;
+    // TODO: consider whether these need to be signed
     ivec4 atlas_offs;
 };
 

voxygen/src/mesh/greedy.rs

@@ -82,7 +82,7 @@ pub type SuspendedMesh<'a> = dyn for<'r> FnOnce(&'r mut ColLightInfo) + 'a;
 /// For an explanation of why we want this, see `SuspendedMesh`.
 pub struct GreedyMesh<'a> {
     atlas: guillotiere::SimpleAtlasAllocator,
-    col_lights_size: Vec2<u32>,
+    col_lights_size: Vec2<u16>,
     max_size: guillotiere::Size,
     suspended: Vec<Box<SuspendedMesh<'a>>>,
 }
@@ -190,7 +190,7 @@ impl<'a> GreedyMesh<'a> {
 
 fn greedy_mesh<'a, M: PartialEq, D: 'a, FL, FG, FO, FS, FP, FT>(
     atlas: &mut guillotiere::SimpleAtlasAllocator,
-    col_lights_size: &mut Vec2<u32>,
+    col_lights_size: &mut Vec2<u16>,
     max_size: guillotiere::Size,
     GreedyConfig {
         mut data,
@@ -428,7 +428,7 @@ fn add_to_atlas(
     norm: Vec3<i16>,
     faces_forward: bool,
     max_size: guillotiere::Size,
-    cur_size: &mut Vec2<u32>,
+    cur_size: &mut Vec2<u16>,
 ) -> guillotiere::Rectangle {
     // TODO: Check this conversion.
     let atlas_rect;
@@ -473,8 +473,8 @@ fn add_to_atlas(
     // a u16 and we never grew the atlas, meaning all valid coordinates within the
     // atlas also fit into a u16.
     *cur_size = Vec2::new(
-        cur_size.x.max(atlas_rect.max.x as u32),
+        cur_size.x.max(atlas_rect.max.x as u16),
-        cur_size.y.max(atlas_rect.max.y as u32),
+        cur_size.y.max(atlas_rect.max.y as u16),
     );
 
     // NOTE: pos can be converted safely from usize to i32 because all legal block

voxygen/src/render/pipelines/mod.rs

@@ -124,11 +124,11 @@ impl Globals {
                 shadow_planes.x,
                 shadow_planes.y,
             ],
+            // TODO: why do we accept values greater than the max?
             light_shadow_count: [
-                // TODO: why do we accept values greater than the max?
+                usize::min(light_count, MAX_POINT_LIGHT_COUNT) as u32,
-                (light_count % (MAX_POINT_LIGHT_COUNT + 1)) as u32,
+                usize::min(shadow_count, MAX_FIGURE_SHADOW_COUNT) as u32,
-                (shadow_count % (MAX_FIGURE_SHADOW_COUNT + 1)) as u32,
+                usize::min(directed_light_count, MAX_DIRECTED_LIGHT_COUNT) as u32,
-                (directed_light_count % (MAX_DIRECTED_LIGHT_COUNT + 1)) as u32,
                 0,
             ],
             shadow_proj_factors: [

voxygen/src/render/pipelines/shadow.rs

@@ -82,8 +82,8 @@ pub fn create_col_lights(
     let texture_info = wgpu::TextureDescriptor {
         label: None,
         size: wgpu::Extent3d {
-            width: col_lights_size.x,
+            width: u32::from(col_lights_size.x),
-            height: col_lights_size.y,
+            height: u32::from(col_lights_size.y),
             depth_or_array_layers: 1,
         },
         mip_level_count: 1,

voxygen/src/render/pipelines/terrain.rs

@@ -141,12 +141,20 @@ impl VertexTrait for Vertex {
 #[derive(Copy, Clone, Debug, Zeroable, Pod)]
 // TODO: new function and private fields??
 pub struct Locals {
-    pub model_offs: [f32; 3],
+    model_offs: [f32; 3],
-    pub load_time: f32,
+    load_time: f32,
-    pub atlas_offs: [i32; 4],
+    atlas_offs: [i32; 4],
 }
 
 impl Locals {
+    pub fn new(model_offs: Vec3<f32>, atlas_offs: Vec2<u32>, load_time: f32) -> Self {
+        Self {
+            model_offs: model_offs.into_array(),
+            load_time,
+            atlas_offs: Vec4::new(atlas_offs.x as i32, atlas_offs.y as i32, 0, 0).into_array(),
+        }
+    }
+
     pub fn default() -> Self {
         Self {
             model_offs: [0.0; 3],

voxygen/src/render/renderer.rs

@@ -38,7 +38,7 @@ use vek::*;
 /// A type representing data that can be converted to an immutable texture map
 /// of ColLight data (used for texture atlases created during greedy meshing).
 // TODO: revert to u16
-pub type ColLightInfo = (Vec<[u8; 4]>, Vec2<u32>);
+pub type ColLightInfo = (Vec<[u8; 4]>, Vec2<u16>);
 
 const QUAD_INDEX_BUFFER_U16_START_VERT_LEN: u16 = 3000;
 const QUAD_INDEX_BUFFER_U32_START_VERT_LEN: u32 = 3000;
@@ -678,7 +678,10 @@ impl Renderer {
         .unwrap_or_else(|| (Vec2::new(1, 1), Vec2::new(1, 1)))
     }
 
-    // TODO: @Sharp what should this look like with wgpu?
+    // TODO: Seamless is potentially the default with wgpu but we need further
+    // investigation into whether this is actually turned on for the OpenGL
+    // backend
+    //
     /// NOTE: Supported by Vulkan (by default), DirectX 10+ (it seems--it's hard
     /// to find proof of this, but Direct3D 10 apparently does it by
     /// default, and 11 definitely does, so I assume it's natively supported

voxygen/src/render/renderer/drawer.rs

@@ -839,10 +839,6 @@ impl<'pass_ref, 'pass: 'pass_ref> PreparedUiDrawer<'pass_ref, 'pass> {
 
     pub fn set_scissor(&mut self, scissor: Aabr<u16>) {
         let Aabr { min, max } = scissor;
-        // TODO: Got an invalid scissor panic from wgpu,
-        // use this if you can reproduce
-        // Note: might have been from changing monitors
-        // dbg!(&scissor)
         self.render_pass.set_scissor_rect(
             min.x as u32,
             min.y as u32,

voxygen/src/scene/terrain.rs

@@ -1088,40 +1088,32 @@ impl<V: RectRasterableVol> Terrain<V> {
                         let atlas = &mut self.atlas;
                         let chunks = &mut self.chunks;
                         let col_lights = &mut self.col_lights;
-                        let allocation = atlas
+                        let alloc_size =
-                            .allocate(guillotiere::Size::new(
+                            guillotiere::Size::new(i32::from(tex_size.x), i32::from(tex_size.y));
-                                tex_size.x as i32, /* TODO: adjust ColLightInfo to avoid the
-                                                    * cast here? */
-                                tex_size.y as i32,
-                            ))
-                            .unwrap_or_else(|| {
-                                // Atlas allocation failure: try allocating a new texture and atlas.
-                                let (new_atlas, new_col_lights) = Self::make_atlas(renderer)
-                                    .expect("Failed to create atlas texture");
 
-                                // We reset the atlas and clear allocations from existing chunks,
+                        let allocation = atlas.allocate(alloc_size).unwrap_or_else(|| {
-                                // even though we haven't yet
+                            // Atlas allocation failure: try allocating a new texture and atlas.
-                                // checked whether the new allocation can fit in
+                            let (new_atlas, new_col_lights) =
-                                // the texture.  This is reasonable because we don't have a fallback
+                                Self::make_atlas(renderer).expect("Failed to create atlas texture");
-                                // if a single chunk can't fit in an empty atlas of maximum size.
-                                //
-                                // TODO: Consider attempting defragmentation first rather than just
-                                // always moving everything into the new chunk.
-                                chunks.iter_mut().for_each(|(_, chunk)| {
-                                    chunk.col_lights_alloc = None;
-                                });
-                                *atlas = new_atlas;
-                                *col_lights = Arc::new(new_col_lights);
 
-                                atlas
+                            // We reset the atlas and clear allocations from existing chunks,
-                                    .allocate(guillotiere::Size::new(
+                            // even though we haven't yet
-                                        tex_size.x as i32, /* TODO: adjust ColLightInfo to avoid
+                            // checked whether the new allocation can fit in
-                                                            * the
+                            // the texture.  This is reasonable because we don't have a fallback
-                                                            * cast here? */
+                            // if a single chunk can't fit in an empty atlas of maximum size.
-                                        tex_size.y as i32,
+                            //
-                                    ))
+                            // TODO: Consider attempting defragmentation first rather than just
-                                    .expect("Chunk data does not fit in a texture of maximum size.")
+                            // always moving everything into the new chunk.
+                            chunks.iter_mut().for_each(|(_, chunk)| {
+                                chunk.col_lights_alloc = None;
                             });
+                            *atlas = new_atlas;
+                            *col_lights = Arc::new(new_col_lights);
+
+                            atlas
+                                .allocate(alloc_size)
+                                .expect("Chunk data does not fit in a texture of maximum size.")
+                        });
 
                         // NOTE: Cast is safe since the origin was a u16.
                         let atlas_offs = Vec2::new(
@@ -1131,7 +1123,7 @@ impl<V: RectRasterableVol> Terrain<V> {
                         renderer.update_texture(
                             &col_lights.texture,
                             atlas_offs.into_array(),
-                            tex_size.into_array(),
+                            tex_size.map(|e| u32::from(e)).into_array(),
                             &tex,
                         );
 
@@ -1144,22 +1136,15 @@ impl<V: RectRasterableVol> Terrain<V> {
                             light_map: mesh.light_map,
                             glow_map: mesh.glow_map,
                             sprite_instances,
-                            locals: renderer.create_terrain_bound_locals(&[TerrainLocals {
+                            locals: renderer.create_terrain_bound_locals(&[TerrainLocals::new(
-                                model_offs: Vec3::from(
+                                Vec3::from(
                                     response.pos.map2(VolGrid2d::<V>::chunk_size(), |e, sz| {
                                         e as f32 * sz as f32
                                     }),
-                                )
+                                ),
-                                .into_array(),
+                                atlas_offs,
-                                atlas_offs: Vec4::new(
-                                    atlas_offs.x as i32,
-                                    atlas_offs.y as i32,
-                                    0,
-                                    0,
-                                )
-                                .into_array(),
                                 load_time,
-                            }]),
+                            )]),
                             visible: Visibility {
                                 in_range: false,
                                 in_frustum: false,