From be39054767c7769b9ffe52f3e153b0d8de1f075a Mon Sep 17 00:00:00 2001 From: Avi Weinstock Date: Sat, 1 May 2021 14:28:20 -0400 Subject: [PATCH] Make terrain compression a checkbox instead of a bandwidth (throughput?) heuristic. --- CHANGELOG.md | 1 + assets/voxygen/i18n/en/hud/hud_settings.ron | 1 + client/src/lib.rs | 11 +++- common/net/src/msg/client.rs | 6 +- common/net/src/msg/mod.rs | 2 +- common/net/src/msg/server.rs | 8 +-- server/src/metrics.rs | 26 ++++---- server/src/presence.rs | 2 + server/src/sys/msg/in_game.rs | 5 ++ server/src/sys/msg/terrain.rs | 30 ++++----- server/src/sys/terrain.rs | 61 ++++++++++--------- server/src/sys/terrain_sync.rs | 12 ++-- voxygen/src/hud/settings_window/video.rs | 38 ++++++++++++ voxygen/src/session/mod.rs | 10 ++- voxygen/src/session/settings_change.rs | 8 +++ voxygen/src/settings/graphics.rs | 2 + .../examples/chunk_compression_benchmarks.rs | 42 ++++++++++++- 17 files changed, 185 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a808c89659..b92d92a37c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Custom map markers can be placed now - Fundamentals/prototype for wiring system - Mountain peak and lake markers on the map +- There's now a checkbox in the graphics tab to opt-in to receiving lossily-compressed terrain colors. ### Changed diff --git a/assets/voxygen/i18n/en/hud/hud_settings.ron b/assets/voxygen/i18n/en/hud/hud_settings.ron index d7c5ffc949..f543e8f2d2 100644 --- a/assets/voxygen/i18n/en/hud/hud_settings.ron +++ b/assets/voxygen/i18n/en/hud/hud_settings.ron @@ -77,6 +77,7 @@ "hud.settings.fullscreen_mode.exclusive": "Exclusive", "hud.settings.fullscreen_mode.borderless": "Borderless", "hud.settings.particles": "Particles", + "hud.settings.lossy_terrain_compression": "Lossy terrain compression", "hud.settings.resolution": "Resolution", "hud.settings.bit_depth": "Bit Depth", "hud.settings.refresh_rate": "Refresh Rate", diff --git a/client/src/lib.rs b/client/src/lib.rs index 2b6d6f8529..72eeca7acf 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -800,7 +800,10 @@ impl Client { | ClientGeneral::RefundSkill(_) | ClientGeneral::RequestSiteInfo(_) | ClientGeneral::UnlockSkillGroup(_) - | ClientGeneral::RequestPlayerPhysics { .. } => &mut self.in_game_stream, + | ClientGeneral::RequestPlayerPhysics { .. } + | ClientGeneral::RequestLossyTerrainCompression { .. } => { + &mut self.in_game_stream + }, //Only in game, terrain ClientGeneral::TerrainChunkRequest { .. } => &mut self.terrain_stream, //Always possible @@ -820,6 +823,12 @@ impl Client { }) } + pub fn request_lossy_terrain_compression(&mut self, lossy_terrain_compression: bool) { + self.send_msg(ClientGeneral::RequestLossyTerrainCompression { + lossy_terrain_compression, + }) + } + fn send_msg(&mut self, msg: S) where S: Into, diff --git a/common/net/src/msg/client.rs b/common/net/src/msg/client.rs index ccf9bdbd32..0da0c8c360 100644 --- a/common/net/src/msg/client.rs +++ b/common/net/src/msg/client.rs @@ -84,6 +84,9 @@ pub enum ClientGeneral { RequestPlayerPhysics { server_authoritative: bool, }, + RequestLossyTerrainCompression { + lossy_terrain_compression: bool, + }, } impl ClientMsg { @@ -121,7 +124,8 @@ impl ClientMsg { | ClientGeneral::RefundSkill(_) | ClientGeneral::RequestSiteInfo(_) | ClientGeneral::UnlockSkillGroup(_) - | ClientGeneral::RequestPlayerPhysics { .. } => { + | ClientGeneral::RequestPlayerPhysics { .. } + | ClientGeneral::RequestLossyTerrainCompression { .. } => { c_type == ClientType::Game && presence.is_some() }, //Always possible diff --git a/common/net/src/msg/mod.rs b/common/net/src/msg/mod.rs index 889782df64..fef7a11cc8 100644 --- a/common/net/src/msg/mod.rs +++ b/common/net/src/msg/mod.rs @@ -15,7 +15,7 @@ pub use self::{ server::{ CharacterInfo, DisconnectReason, InviteAnswer, Notification, PlayerInfo, PlayerListUpdate, RegisterError, SerializedTerrainChunk, ServerGeneral, ServerInfo, ServerInit, ServerMsg, - ServerRegisterAnswer, TERRAIN_LOW_BANDWIDTH, + ServerRegisterAnswer, }, world_msg::WorldMapMsg, }; diff --git a/common/net/src/msg/server.rs b/common/net/src/msg/server.rs index 45c53e4f9f..a35cee86fc 100644 --- a/common/net/src/msg/server.rs +++ b/common/net/src/msg/server.rs @@ -73,13 +73,9 @@ pub enum SerializedTerrainChunk { TriPng(WireChonk, WidePacking, TerrainChunkMeta, TerrainChunkSize>), } -/// If someone has less than this number of bytes per second of bandwidth, spend -/// more CPU generating a smaller encoding of terrain data. -pub const TERRAIN_LOW_BANDWIDTH: f32 = 500_000.0; - impl SerializedTerrainChunk { - pub fn via_heuristic(chunk: &TerrainChunk, low_bandwidth: bool) -> Self { - if low_bandwidth && (chunk.get_max_z() - chunk.get_min_z() <= 128) { + pub fn via_heuristic(chunk: &TerrainChunk, lossy_compression: bool) -> Self { + if lossy_compression && (chunk.get_max_z() - chunk.get_min_z() <= 128) { Self::quadpng(chunk) } else { Self::deflate(chunk) diff --git a/server/src/metrics.rs b/server/src/metrics.rs index c58538c96f..f20107c74d 100644 --- a/server/src/metrics.rs +++ b/server/src/metrics.rs @@ -34,8 +34,8 @@ pub struct NetworkRequestMetrics { pub chunks_request_dropped: IntCounter, pub chunks_served_from_memory: IntCounter, pub chunks_generation_triggered: IntCounter, - pub chunks_served_lo_bandwidth: IntCounter, - pub chunks_served_hi_bandwidth: IntCounter, + pub chunks_served_lossy: IntCounter, + pub chunks_served_lossless: IntCounter, } pub struct ChunkGenMetrics { @@ -189,29 +189,27 @@ impl NetworkRequestMetrics { "chunks_generation_triggered", "number of all chunks that were requested and needs to be generated", ))?; - let chunks_served_lo_bandwidth = IntCounter::with_opts(Opts::new( - "chunks_served_lo_bandwidth", - "number of chunks that were sent with compression setting by the low bandwidth \ - heuristic", + let chunks_served_lossy = IntCounter::with_opts(Opts::new( + "chunks_served_lossy", + "number of chunks that were sent with lossy compression requested", ))?; - let chunks_served_hi_bandwidth = IntCounter::with_opts(Opts::new( - "chunks_served_hi_bandwidth", - "number of chunks that were sent with compression setting by the high bandwidth \ - heuristic", + let chunks_served_lossless = IntCounter::with_opts(Opts::new( + "chunks_served_lossless", + "number of chunks that were sent with lossless compression requested", ))?; registry.register(Box::new(chunks_request_dropped.clone()))?; registry.register(Box::new(chunks_served_from_memory.clone()))?; registry.register(Box::new(chunks_generation_triggered.clone()))?; - registry.register(Box::new(chunks_served_lo_bandwidth.clone()))?; - registry.register(Box::new(chunks_served_hi_bandwidth.clone()))?; + registry.register(Box::new(chunks_served_lossy.clone()))?; + registry.register(Box::new(chunks_served_lossless.clone()))?; Ok(Self { chunks_request_dropped, chunks_served_from_memory, chunks_generation_triggered, - chunks_served_lo_bandwidth, - chunks_served_hi_bandwidth, + chunks_served_lossy, + chunks_served_lossless, }) } } diff --git a/server/src/presence.rs b/server/src/presence.rs index 08ea3c7598..7c6df973a7 100644 --- a/server/src/presence.rs +++ b/server/src/presence.rs @@ -9,6 +9,7 @@ use vek::*; pub struct Presence { pub view_distance: u32, pub kind: PresenceKind, + pub lossy_terrain_compression: bool, } impl Presence { @@ -16,6 +17,7 @@ impl Presence { Self { view_distance, kind, + lossy_terrain_compression: false, } } } diff --git a/server/src/sys/msg/in_game.rs b/server/src/sys/msg/in_game.rs index 149ddfcd46..9aa4e3ee40 100644 --- a/server/src/sys/msg/in_game.rs +++ b/server/src/sys/msg/in_game.rs @@ -253,6 +253,11 @@ impl Sys { setting.client_optin = server_authoritative; } }, + ClientGeneral::RequestLossyTerrainCompression { + lossy_terrain_compression, + } => { + presence.lossy_terrain_compression = lossy_terrain_compression; + }, _ => tracing::error!("not a client_in_game msg"), } Ok(()) diff --git a/server/src/sys/msg/terrain.rs b/server/src/sys/msg/terrain.rs index 81ac0e2c89..495e280481 100644 --- a/server/src/sys/msg/terrain.rs +++ b/server/src/sys/msg/terrain.rs @@ -6,9 +6,7 @@ use common::{ vol::RectVolSize, }; use common_ecs::{Job, Origin, ParMode, Phase, System}; -use common_net::msg::{ - ClientGeneral, SerializedTerrainChunk, ServerGeneral, TERRAIN_LOW_BANDWIDTH, -}; +use common_net::msg::{ClientGeneral, SerializedTerrainChunk, ServerGeneral}; use rayon::iter::ParallelIterator; use specs::{Entities, Join, ParJoin, Read, ReadExpect, ReadStorage}; use tracing::{debug, trace}; @@ -79,21 +77,17 @@ impl<'a> System<'a> for Sys { match terrain.get_key_arc(key) { Some(chunk) => { network_metrics.chunks_served_from_memory.inc(); - if let Some(participant) = &client.participant { - let low_bandwidth = - participant.bandwidth() < TERRAIN_LOW_BANDWIDTH; - client.send(ServerGeneral::TerrainChunkUpdate { - key, - chunk: Ok(SerializedTerrainChunk::via_heuristic( - &chunk, - low_bandwidth, - )), - })?; - if low_bandwidth { - network_metrics.chunks_served_lo_bandwidth.inc(); - } else { - network_metrics.chunks_served_hi_bandwidth.inc(); - } + client.send(ServerGeneral::TerrainChunkUpdate { + key, + chunk: Ok(SerializedTerrainChunk::via_heuristic( + &chunk, + presence.lossy_terrain_compression, + )), + })?; + if presence.lossy_terrain_compression { + network_metrics.chunks_served_lossy.inc(); + } else { + network_metrics.chunks_served_lossless.inc(); } }, None => { diff --git a/server/src/sys/terrain.rs b/server/src/sys/terrain.rs index 8c6abc419e..b352de9438 100644 --- a/server/src/sys/terrain.rs +++ b/server/src/sys/terrain.rs @@ -14,7 +14,7 @@ use common::{ LoadoutBuilder, SkillSetBuilder, }; use common_ecs::{Job, Origin, Phase, System}; -use common_net::msg::{SerializedTerrainChunk, ServerGeneral, TERRAIN_LOW_BANDWIDTH}; +use common_net::msg::{SerializedTerrainChunk, ServerGeneral}; use common_state::TerrainChanges; use comp::Behavior; use specs::{Join, Read, ReadExpect, ReadStorage, Write, WriteExpect}; @@ -43,34 +43,35 @@ impl LazyTerrainMessage { &mut self, network_metrics: &NetworkRequestMetrics, client: &Client, + presence: &Presence, chunk_key: &vek::Vec2, generate_chunk: F, ) -> Result<(), A> { - if let Some(participant) = &client.participant { - let low_bandwidth = participant.bandwidth() < TERRAIN_LOW_BANDWIDTH; - let lazy_msg = if low_bandwidth { - &mut self.lazy_msg_lo - } else { - &mut self.lazy_msg_hi - }; - if lazy_msg.is_none() { - *lazy_msg = Some(client.prepare(ServerGeneral::TerrainChunkUpdate { - key: *chunk_key, - chunk: Ok(match generate_chunk() { - Ok(chunk) => SerializedTerrainChunk::via_heuristic(&chunk, low_bandwidth), - Err(e) => return Err(e), - }), - })); - } - lazy_msg.as_ref().map(|ref msg| { - let _ = client.send_prepared(&msg); - if low_bandwidth { - network_metrics.chunks_served_lo_bandwidth.inc(); - } else { - network_metrics.chunks_served_hi_bandwidth.inc(); - } - }); + let lazy_msg = if presence.lossy_terrain_compression { + &mut self.lazy_msg_lo + } else { + &mut self.lazy_msg_hi + }; + if lazy_msg.is_none() { + *lazy_msg = Some(client.prepare(ServerGeneral::TerrainChunkUpdate { + key: *chunk_key, + chunk: Ok(match generate_chunk() { + Ok(chunk) => SerializedTerrainChunk::via_heuristic( + &chunk, + presence.lossy_terrain_compression, + ), + Err(e) => return Err(e), + }), + })); } + lazy_msg.as_ref().map(|ref msg| { + let _ = client.send_prepared(&msg); + if presence.lossy_terrain_compression { + network_metrics.chunks_served_lossy.inc(); + } else { + network_metrics.chunks_served_lossless.inc(); + } + }); Ok(()) } } @@ -293,9 +294,13 @@ impl<'a> System<'a> for Sys { if adjusted_dist_sqr <= presence.view_distance.pow(2) { lazy_msg - .prepare_and_send::(&network_metrics, &client, &key, || { - Ok(&*chunk) - }) + .prepare_and_send::( + &network_metrics, + &client, + &presence, + &key, + || Ok(&*chunk), + ) .into_ok(); } }); diff --git a/server/src/sys/terrain_sync.rs b/server/src/sys/terrain_sync.rs index eab170e9ec..be78f63594 100644 --- a/server/src/sys/terrain_sync.rs +++ b/server/src/sys/terrain_sync.rs @@ -35,11 +35,13 @@ impl<'a> System<'a> for Sys { for (presence, pos, client) in (&presences, &positions, &clients).join() { if super::terrain::chunk_in_vd(pos.0, *chunk_key, &terrain, presence.view_distance) { - if let Err(()) = - lazy_msg.prepare_and_send(&network_metrics, &client, chunk_key, || { - terrain.get_key(*chunk_key).ok_or(()) - }) - { + if let Err(()) = lazy_msg.prepare_and_send( + &network_metrics, + &client, + &presence, + chunk_key, + || terrain.get_key(*chunk_key).ok_or(()), + ) { break 'chunk; } } diff --git a/voxygen/src/hud/settings_window/video.rs b/voxygen/src/hud/settings_window/video.rs index 9f697d9c66..835bb1eb69 100644 --- a/voxygen/src/hud/settings_window/video.rs +++ b/voxygen/src/hud/settings_window/video.rs @@ -82,6 +82,8 @@ widget_ids! { // particles_button, particles_label, + lossy_terrain_compression_button, + lossy_terrain_compression_label, // fullscreen_button, fullscreen_label, @@ -874,6 +876,42 @@ impl<'a> Widget for Video<'a> { events.push(ToggleParticlesEnabled(particles_enabled)); } + // Lossy terrain compression + Text::new( + &self + .localized_strings + .get("hud.settings.lossy_terrain_compression"), + ) + .font_size(self.fonts.cyri.scale(14)) + .font_id(self.fonts.cyri.conrod_id) + .right_from(state.ids.particles_label, 64.0) + .color(TEXT_COLOR) + .set(state.ids.lossy_terrain_compression_label, ui); + + let lossy_terrain_compression = ToggleButton::new( + self.global_state + .settings + .graphics + .lossy_terrain_compression, + self.imgs.checkbox, + self.imgs.checkbox_checked, + ) + .w_h(18.0, 18.0) + .right_from(state.ids.lossy_terrain_compression_label, 10.0) + .hover_images(self.imgs.checkbox_mo, self.imgs.checkbox_checked_mo) + .press_images(self.imgs.checkbox_press, self.imgs.checkbox_checked) + .set(state.ids.lossy_terrain_compression_button, ui); + + if self + .global_state + .settings + .graphics + .lossy_terrain_compression + != lossy_terrain_compression + { + events.push(ToggleLossyTerrainCompression(lossy_terrain_compression)); + } + // Resolution let resolutions: Vec<[u16; 2]> = state .video_modes diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs index 57f293f0d7..85e3287966 100644 --- a/voxygen/src/session/mod.rs +++ b/voxygen/src/session/mod.rs @@ -88,9 +88,13 @@ impl SessionState { scene .camera_mut() .set_fov_deg(global_state.settings.graphics.fov); - client - .borrow_mut() - .request_player_physics(global_state.settings.gameplay.player_physics_behavior); + { + let mut client = client.borrow_mut(); + client.request_player_physics(global_state.settings.gameplay.player_physics_behavior); + client.request_lossy_terrain_compression( + global_state.settings.graphics.lossy_terrain_compression, + ); + } let hud = Hud::new(global_state, &client.borrow()); let walk_forward_dir = scene.camera().forward_xy(); let walk_right_dir = scene.camera().right_xy(); diff --git a/voxygen/src/session/settings_change.rs b/voxygen/src/session/settings_change.rs index c9f4a87fd6..53c2d24c7b 100644 --- a/voxygen/src/session/settings_change.rs +++ b/voxygen/src/session/settings_change.rs @@ -71,6 +71,7 @@ pub enum Graphics { ChangeFullscreenMode(FullScreenSettings), ToggleParticlesEnabled(bool), + ToggleLossyTerrainCompression(bool), AdjustWindowSize([u16; 2]), ResetGraphicsSettings, @@ -329,6 +330,13 @@ impl SettingsChange { Graphics::ToggleParticlesEnabled(particles_enabled) => { settings.graphics.particles_enabled = particles_enabled; }, + Graphics::ToggleLossyTerrainCompression(lossy_terrain_compression) => { + settings.graphics.lossy_terrain_compression = lossy_terrain_compression; + session_state + .client + .borrow_mut() + .request_lossy_terrain_compression(lossy_terrain_compression); + }, Graphics::AdjustWindowSize(new_size) => { global_state.window.set_size(new_size.into()); settings.graphics.window_size = new_size; diff --git a/voxygen/src/settings/graphics.rs b/voxygen/src/settings/graphics.rs index 3256d72d4a..1155a7a131 100644 --- a/voxygen/src/settings/graphics.rs +++ b/voxygen/src/settings/graphics.rs @@ -32,6 +32,7 @@ pub struct GraphicsSettings { pub view_distance: u32, pub sprite_render_distance: u32, pub particles_enabled: bool, + pub lossy_terrain_compression: bool, pub figure_lod_render_distance: u32, pub max_fps: Fps, pub fov: u16, @@ -50,6 +51,7 @@ impl Default for GraphicsSettings { view_distance: 10, sprite_render_distance: 100, particles_enabled: true, + lossy_terrain_compression: false, figure_lod_render_distance: 300, max_fps: Fps::Max(60), fov: 70, diff --git a/world/examples/chunk_compression_benchmarks.rs b/world/examples/chunk_compression_benchmarks.rs index 1397e6db84..2470a69c0a 100644 --- a/world/examples/chunk_compression_benchmarks.rs +++ b/world/examples/chunk_compression_benchmarks.rs @@ -563,7 +563,7 @@ fn main() { .unwrap(), )); - const SKIP_DEFLATE_2_5: bool = true; + const SKIP_DEFLATE_2_5: bool = false; const SKIP_DYNA: bool = true; const SKIP_IMAGECHONK: bool = true; const SKIP_MIXED: bool = true; @@ -701,6 +701,42 @@ fn main() { ("deflate4chonk", (deflate4chonk_post - deflate4chonk_pre).subsec_nanos()), ("deflate5chonk", (deflate5chonk_post - deflate5chonk_pre).subsec_nanos()), ]); + { + let bucket = z_buckets + .entry("deflate2") + .or_default() + .entry(chunk.get_max_z() - chunk.get_min_z()) + .or_insert((0, 0.0)); + bucket.0 += 1; + bucket.1 += (deflate2chonk_post - deflate2chonk_pre).subsec_nanos() as f32; + } + { + let bucket = z_buckets + .entry("deflate3") + .or_default() + .entry(chunk.get_max_z() - chunk.get_min_z()) + .or_insert((0, 0.0)); + bucket.0 += 1; + bucket.1 += (deflate3chonk_post - deflate3chonk_pre).subsec_nanos() as f32; + } + { + let bucket = z_buckets + .entry("deflate4") + .or_default() + .entry(chunk.get_max_z() - chunk.get_min_z()) + .or_insert((0, 0.0)); + bucket.0 += 1; + bucket.1 += (deflate4chonk_post - deflate4chonk_pre).subsec_nanos() as f32; + } + { + let bucket = z_buckets + .entry("deflate5") + .or_default() + .entry(chunk.get_max_z() - chunk.get_min_z()) + .or_insert((0, 0.0)); + bucket.0 += 1; + bucket.1 += (deflate5chonk_post - deflate5chonk_pre).subsec_nanos() as f32; + } } if !SKIP_DYNA { @@ -886,7 +922,7 @@ fn main() { ("tripngaverage", (tripngaverage_post - tripngaverage_pre).subsec_nanos()), ("tripngconst", (tripngconst_post - tripngconst_pre).subsec_nanos()), ]); - { + if false { let bucket = z_buckets .entry("quadpngquarttall") .or_default() @@ -906,7 +942,7 @@ fn main() { bucket.1 += (quadpngquartwide_post - quadpngquartwide_pre).subsec_nanos() as f32; } - if true { + if false { let bucket = z_buckets .entry("tripngaverage") .or_default()