Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zero size crash #276

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions maplibre-winit/src/input/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl InputController {
}

pub trait UpdateState {
/// Assumes that the view state is drawable. Implementations may panic otherwise.
fn update_state(&mut self, state: &mut MapContext, dt: Duration);
}

Expand Down
4 changes: 3 additions & 1 deletion maplibre-winit/src/input/pan_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ impl UpdateState for PanHandler {
if let (Some(window_position), Some(start_window_position)) =
(self.window_position, self.start_window_position)
{
let view_proj = view_state.view_projection();
let view_proj = view_state
.view_projection()
.expect("View state must have a valid for pan handler.");
let inverted_view_proj = view_proj.invert();

let delta = if let (Some(start), Some(current)) = (
Expand Down
4 changes: 3 additions & 1 deletion maplibre-winit/src/input/query_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl UpdateState for QueryHandler {
) {
if self.clicking {
if let Some(window_position) = self.window_position {
let view_proj = view_state.view_projection();
let view_proj = view_state
.view_projection()
.expect("View projection must be valid for query handler.");
let inverted_view_proj = view_proj.invert();

let z = view_state.visible_level(); // FIXME: can be wrong, if tiles of different z are visible
Expand Down
4 changes: 3 additions & 1 deletion maplibre-winit/src/input/zoom_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ impl UpdateState for ZoomHandler {
view_state.update_zoom(next_zoom);
self.zoom_delta = None;

let view_proj = view_state.view_projection();
let view_proj = view_state
.view_projection()
.expect("View projection must be valid for zoom handler.");
let inverted_view_proj = view_proj.invert();

if let Some(cursor_position) = view_state.camera().window_to_world_at_ground(
Expand Down
6 changes: 5 additions & 1 deletion maplibre-winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ impl<ET: 'static + PartialEq + Debug> EventLoop<ET> for WinitEventLoop<ET> {
last_render_time = now;

if let Ok(map_context) = map.context_mut() {
input_controller.update_state(map_context, dt);
if map_context.view_state.is_drawable() {
input_controller.update_state(map_context, dt);
} else {
log::trace!("Re-draw requested although map context is not drawable.");
}
}

// TODO: Handle gracefully
Expand Down
17 changes: 16 additions & 1 deletion maplibre/src/render/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ pub const FLIP_Y: Matrix4<f64> = Matrix4::new(
0.0, 0.0, 0.0, 1.0,
);

/// Guaranteed to be invertible
#[derive(Debug)]
pub struct ViewProjection(Matrix4<f64>);

impl ViewProjection {
#[tracing::instrument(skip_all)]
pub fn invert(&self) -> InvertedViewProjection {
InvertedViewProjection(self.0.invert().expect("Unable to invert view projection"))
InvertedViewProjection(
self.0
.invert()
.expect("ViewProjections must be invertible."),
)
}

pub fn project(&self, vector: Vector4<f64>) -> Vector4<f64> {
Expand Down Expand Up @@ -69,6 +74,8 @@ impl ModelViewProjection {
const MIN_PITCH: Rad<f64> = Rad(-0.5);
const MAX_PITCH: Rad<f64> = Rad(0.5);

/// A camera that is always valid and has a non-zero size. This means that the view projection is
/// always invertible.
#[derive(Debug, Clone)]
pub struct Camera {
position: Point3<f64>, // The z axis never changes, the zoom is used instead
Expand Down Expand Up @@ -106,7 +113,11 @@ impl Camera {
}
}

/// Panics:
/// width and height must both be non-zero.
pub fn resize(&mut self, width: u32, height: u32) {
assert_ne!(width, 0, "Cannot set camera width to zero");
assert_ne!(height, 0, "Cannot set camera height to zero");
self.width = width as f64;
self.height = height as f64;
}
Expand Down Expand Up @@ -411,7 +422,11 @@ impl Perspective {
}
}

/// Panics:
/// Panics if width or height are non-zero to ensure a valid projection
pub fn resize(&mut self, width: u32, height: u32) {
assert_ne!(width, 0, "Cannot set perspective width to zero");
assert_ne!(height, 0, "Cannot set perspective height to zero");
self.current_projection = Self::calc_matrix(
width as f64 / height as f64,
self.fovy,
Expand Down
29 changes: 12 additions & 17 deletions maplibre/src/render/resource/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ pub enum Head {
}

pub struct Surface {
size: WindowSize,
/// Only render if size is not None
size: Option<WindowSize>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable. I think on android this will be initially None.

head: Head,
}

Expand Down Expand Up @@ -193,7 +194,7 @@ impl Surface {
log::info!("format features: {texture_format_features:?}");

Self {
size,
size: Some(size),
head: Head::Headed(WindowHead {
surface,
size,
Expand Down Expand Up @@ -247,7 +248,7 @@ impl Surface {
let texture = device.create_texture(&texture_descriptor);

Self {
size,
size: Some(size),
head: Head::Headless(Arc::new(BufferedTextureHead {
texture,
texture_format: format,
Expand Down Expand Up @@ -289,22 +290,19 @@ impl Surface {
}
}

pub fn size(&self) -> WindowSize {
pub fn size(&self) -> Option<WindowSize> {
self.size
}

pub fn resize(&mut self, width: u32, height: u32) {
self.size = WindowSize::new(width, height).expect("Invalid size for resizing the surface.");
self.size = WindowSize::new(width, height);
}

pub fn reconfigure(&mut self, device: &wgpu::Device) {
match &mut self.head {
Head::Headed(window) => {
if window.has_changed(&(self.size.width(), self.size.height())) {
window.resize_and_configure(self.size.width(), self.size.height(), device);
}
if let (Some(size), Head::Headed(window_head)) = (self.size, &mut self.head) {
if window_head.has_changed(&(size.width(), size.height())) {
window_head.resize_and_configure(size.width(), size.height(), device);
}
Head::Headless(_) => {}
}
}

Expand All @@ -316,13 +314,10 @@ impl Surface {
where
MW: MapWindow + HeadedMapWindow,
{
match &mut self.head {
Head::Headed(window_head) => {
if window_head.has_changed(&(self.size.width(), self.size.height())) {
window_head.recreate_surface(window, instance)?;
}
if let (Some(size), Head::Headed(window_head)) = (self.size, &mut self.head) {
if window_head.has_changed(&(size.width(), size.height())) {
window_head.recreate_surface(window, instance)?;
}
Head::Headless(_) => {}
}
Ok(())
}
Expand Down
4 changes: 3 additions & 1 deletion maplibre/src/render/systems/resource_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ impl System for ResourceSystem {

let surface = &mut state.surface;

let size = surface.size();
let Some(size) = surface.size() else {
return;
};

surface.reconfigure(device);

Expand Down
6 changes: 5 additions & 1 deletion maplibre/src/render/systems/upload_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ pub fn upload_system(
..
}: &mut MapContext,
) {
let Some(view_proj) = view_state.view_projection() else {
// skip every thing if there is no view.
return;
};

let Some(Initialized(tile_view_pattern)) = world
.resources
.query_mut::<&mut Eventually<WgpuTileViewPattern>>()
else {
return;
};

let view_proj = view_state.view_projection();
tile_view_pattern.upload_pattern(queue, &view_proj);
}
30 changes: 25 additions & 5 deletions maplibre/src/view_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ pub struct ViewState {
zoom: ChangeObserver<Zoom>,
camera: ChangeObserver<Camera>,
perspective: Perspective,

// tracks if the view is currently valid
has_finite_size: bool,
}

impl ViewState {
Expand Down Expand Up @@ -55,17 +58,25 @@ impl ViewState {
zoom: ChangeObserver::new(zoom),
camera: ChangeObserver::new(camera),
perspective,
has_finite_size: true,
}
}

pub fn resize(&mut self, width: u32, height: u32) {
self.perspective.resize(width, height);
self.camera.resize(width, height);
if width == 0 || height == 0 {
self.has_finite_size = false;
} else {
self.perspective.resize(width, height);
self.camera.resize(width, height);
self.has_finite_size = true;
}
}

pub fn create_view_region(&self) -> Option<ViewRegion> {
let view_proj = self.view_projection()?;

self.camera
.view_region_bounding_box(&self.view_projection().invert())
.view_region_bounding_box(&view_proj.invert())
.map(|bounding_box| {
ViewRegion::new(
bounding_box,
Expand All @@ -77,8 +88,13 @@ impl ViewState {
})
}

pub fn view_projection(&self) -> ViewProjection {
self.camera.calc_view_proj(&self.perspective)
pub fn view_projection(&self) -> Option<ViewProjection> {
if self.has_finite_size {
Some(self.camera.calc_view_proj(&self.perspective))
} else {
// Cannot create a valid view projection when the view is zero sized.
None
}
}

pub fn visible_level(&self) -> ZoomLevel {
Expand Down Expand Up @@ -114,4 +130,8 @@ impl ViewState {
self.camera.update_reference();
self.zoom.update_reference();
}

pub fn is_drawable(&self) -> bool {
self.has_finite_size
}
}
Loading