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: fixes #68 accept terra objects in addRasterRGB #81

Merged
merged 4 commits into from
Jun 1, 2024

Conversation

trafficonese
Copy link
Contributor

@trafficonese trafficonese commented May 29, 2024

More info in issue #68.

For now I am using the near method for reprojecting.

Should we expose method as new argument and use this leaflet logic
https://github.com/trafficonese/leaflet/blob/92bc272caa9a268140e75ede1966bcdc7d585636/R/layers.R#L499-L513 ?


Here's a little Shiny-App as playground:

Click me
library(shiny)  
library(leafem)
library(leaflet)
library(terra)

rasterdf <- plainview::poppendorf
terrradf <- terra::rast(df)

ui <- fluidPage(
  leafletOutput("map", height = 900)
)

server <- function(input, output, session) {
  output$map <- renderLeaflet({
    leaflet() %>%
      addTiles(group = "OpenStreetMap") %>%
      addRasterRGB(rasterdf, 4,3,2, group="raster") %>% 
      addRasterRGB(terrradf, 4,3,2, group="terra") %>% 
      addLayersControl(overlayGroups = c("raster","terra"))
  })
}
shinyApp(ui, server)

@trafficonese trafficonese changed the title fix: fixes #68 fix: fixes #68 accept terra objects in addRasterRGB May 29, 2024
@tim-salabim
Copy link
Member

Thanks! Yes, I think we should expose the method argument and handle it equivalently to leaflet.

@trafficonese
Copy link
Contributor Author

I included the method argument.

2 other "problems" came up..

  • With terra objects the data will get projected twice currently, also if the data is already in EPSG:3857, as the default value for addRasterImage is project = TRUE.

  • When using a stars object in addRasterRGB we still get the error in Could addRasterRGB support SpatRaster class object #68 because of different lengths:

Error in dim(tileData) <- c(4, nrow(projected), ncol(projected)) :
dims [product 118332] do not match the length of object [110888]

library(leafem); library(leaflet); library(stars)
leaflet() %>% addTiles() %>%
  addRasterRGB(st_as_stars(plainview::poppendorf), 4, project = TRUE)

@trafficonese
Copy link
Contributor Author

With 8c1987e I now check if the projection is already 3857 and set project = FALSE for terra objects.

I also changed apply(mat, 1, anyNA) with rowSums(is.na(mat)) > 0, which should be around 50 times faster. We could further increase the performance with matrixStats::rowAnyNAs(mat), but I think thats fast enough and there is a maximum raster size anyway

Error in addRasterImage_SpatRaster: Raster image too large; 5725014 bytes is greater than maximum 4194304 bytes

@tim-salabim tim-salabim merged commit 7022b73 into r-spatial:master Jun 1, 2024
5 checks passed
@tim-salabim
Copy link
Member

Thanks!

@tim-salabim
Copy link
Member

with 8ab848f reprojecting should now work properly for all 3 classes (raster, terra, stars). I decided to use their native reprojection versions, rather than relying on leaflet::projectRasterForLeaflet()

@trafficonese trafficonese deleted the rgb_terra branch June 1, 2024 11:14
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.

2 participants