Added basic support for choosing the product and sector of imagery from the configuration. #24
Conversation
…om the configuration.
|
Hey, at least you didn't |
|
|
||
| pub fn tile_image(self) -> Image<Box<[u8]>, 3> { | ||
| Image::alloc(self.tile_size(), self.tile_size()).boxed() | ||
| pub fn tile_image(self, sector: &String) -> Image<Box<[u8]>, 3> { |
There was a problem hiding this comment.
It honestly doesn't matter here, but since you said you're new to Rust - it'd be more idiomatic to take an &str (a reference to a string slice, which may be constant/static/refer to a subset of another string) rather than a &String (a reference to a heap-allocated/owned string buffer.) A String can be freely coerced to a &str, but the inverse is not true.
There was a problem hiding this comment.
(If you want to get really fancy, there's also impl AsRef<str> (e.x. sector: impl AsRef<str>) - that makes your function generic over any type that can be cheaply dereferenced to a &str. But that's way overkill for here - just a thing to remember if you ever write some library code.)
Yeah, the current implementation is a bit messy. I may roll it back to the unoptimized-but-easier-to-grok implementation I originally had if I can't get the |
Colonial-Dev
left a comment
Colonial-Dev
left a comment
There was a problem hiding this comment.
Looks like you forgot a conversion call - see the failed run - but again, this otherwise looks great!
3 participants
I just found this project and loved the idea! But I also wanted to see if I could choose which products and sectors from the imagery to use. I saw that this was the [one open issue on the repo, #23. So I took a stab at implementing it.
I will note that this is the first time I have ever looked at a piece of Rust code, so it's entirely possible that I've done something dumb here, and I welcome any comments. But it does seem to work for me at least.
It moves some constants to be optional parameters (with the old values as defaults), and updates the tile size calculation to account for the fact that different sectors have different tile sizes. (I pulled those numbers from the javascript on the SLIDER website.)
There's still a small issue with, for instance, the
conussector. The data isn't square likefull_disk, and so it arrives with black bars on the top/bottom. I don't fully understand how the program is doing the cropping/resizing/compositing, so I haven't tried to fix that yet. Plus at least in my case, it doesn't really matter, since I just center the wallpaper and the margins go off-screen. But it would be nice to be able to output the image at the right aspect ratio.Also, it would be nice to add some validity checks on the names of the sectors and products. They vary by which satellite you choose of course. Right now if you choose an invalid option, the web request just fails with a 404.
Hopefully this can help folks!