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

Feature raster tiles #141

Closed
wants to merge 20 commits into from
Closed

Conversation

quillasp
Copy link

@quillasp quillasp commented Jun 29, 2022

This is the first step of the raster tiles rendering feature #71. There's:

  • Raster style layer from the raster layer mapbox specification,
  • Raster layer that will hold the raster data
  • Two differents sources for vector tiles (Tessellate source) and raster source that will load different data following the layer type.

💻 Examples

🚨 Test instructions

✔️ PR Todo

  • Discussing how the different layer types should be fetched (one source client for the vector tiles and another one for the raster tiles? or fetching from the same source client with different address based on the layer type)
  • Raster data loading and storing
  • Raster rendering

Quillasp added 2 commits June 22, 2022 22:19
This is the first step of the implementation of the raster tile rendering

- Addition of a RasterLayer in the StoredLayer
- Addition of a Source trait for loading TessellateSource and RasterSource
// }
// true
// }
pub fn is_layers_missing(
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably rename to missed_layers. Clean up the commented code.

{
fn load(request_stage: &RequestStage<SM, HC>, coords: &WorldTileCoords, request_id: u32) {
// Sould have a RasterSourceClient or should fetching happen depending
// on the tile type ?
Copy link
Contributor

Choose a reason for hiding this comment

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

You should pass URL to http client from where you want to load a data.

})
}))
.unwrap();
// let client = SourceClient::Http(self.http_source_client.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

Clean up commented code.

maxzoom: None,
minzoom: None,
metadata: None,
paint: Some(LayerPaint::Raster(RasterLayer::default())),
Copy link
Contributor

Choose a reason for hiding this comment

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

paint should be None.

@@ -39,17 +66,20 @@ pub enum LayerPaint {
Line(LinePaint),
#[serde(rename = "fill")]
Fill(FillPaint),
#[serde(rename = "raster")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Clean up this changes. We will not use LayerPaint for raster tiles.

The different sources handles the URL creation based on the layer type
@@ -1,5 +1,6 @@
//! Vector tile layer drawing utilities.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like changes in this file is not used. Please clean up.

Comment on lines 71 to 77
let tile_coords = coords.into_tile(TileAddressingScheme::TMS).unwrap();
let url = format!(
"https://maps.tuerantuer.org/europe_germany/{z}/{x}/{y}.pbf",
x = tile_coords.x,
y = tile_coords.y,
z = tile_coords.z
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This transformation should happen within the source client.

@@ -47,9 +47,9 @@ impl<HC> SourceClient<HC>
where
HC: HttpClient,
{
pub async fn fetch(&self, coords: &WorldTileCoords) -> Result<Vec<u8>, Error> {
pub async fn fetch(&self, url: &str) -> Result<Vec<u8>, Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous interface should be kept. What was the reason for the change?

@@ -77,6 +79,85 @@ where
}
}

pub trait Source<SM, HC>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Source should not be part of the stages module

HC: HttpClient,
{
fn load(
request_stage: &RequestStage<SM, HC>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

A source should not depend on the RequestStage and vice-versa

@@ -29,6 +30,32 @@ pub struct LinePaint {
// TODO a lot
}

pub enum PaintType {
Argb(Alpha<EncodedSrgb<f32>>),
Raster(RasterLayer),
Copy link
Collaborator

Choose a reason for hiding this comment

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

A raster layer is not related to the color. The a raster layer should be another case for the LayerPaint enum.

quillasp and others added 8 commits July 6, 2022 18:18
- Source is in io module and does not depend on the RequestStage anymore
- There's a raster tile pipeline processing
- Source type now load data depending on the source (raster or vector tile)
- Source client fetch data based on the source type
# Conflicts:
#	maplibre-demo/src/main.rs
#	maplibre/src/io/source_client.rs
#	maplibre/src/io/tile_repository.rs
#	maplibre/src/stages/mod.rs
#	maplibre/src/stages/request_stage.rs
#	maplibre/src/style/layer.rs
#	maplibre/src/style/style.rs
@maxammann
Copy link
Collaborator

@quillasp I merged in main and fixed all conflicts

Quillasp and others added 9 commits July 23, 2022 23:27
- Using the `maplibre::render::resource::texture::Texture` instead of creating one directly from the device

- Changing the `ShaderTextureVertex` coordinates

The rendering is still not correct but there is something on the screen at least
The solution is nearing, the reaster tile renders upside down and the vertices coordinates still must be determined
@maxammann
Copy link
Collaborator

@quillasp I would propose not to try to merge main into this PR, but manually check the relevant changes and then copy them to a new branch.
A lot has changed in the past months, so there will definitely be a lot of changes.

@maxammann
Copy link
Collaborator

Closing in favor of #205

@maxammann maxammann closed this Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants