Leaflet.DistortableImage
Leaflet.DistortableImage copied to clipboard
publiclab
Fix export links
Fixes #231
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
- [x] PR is descriptively titled 📑 and links the original issue above 🔗
- [x] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with
grunt - [x] code is in uniquely-named feature branch and has no merge conflicts 📁
- [x] screenshots/GIFs are attached 📎 in case of UI updation
- [x] @mention the original creator of the issue in a comment below for help or for a review
We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!
If tests do fail, click on the red X to learn why by reading the logs.
Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!
@rexagod I don't know much about the full res exporter.. this is how its supposed to work? It takes in the same image as the option? If so we can merge this
@sashadev-sky Notice the fullResolutionSrc property that gets passed from the html file, and the code snippet itself specifies the source, so I guess we're good here.
https://github.com/publiclab/Leaflet.DistortableImage/blob/ec123048635660e1f79ed30ceaf2747e7e664e2e/src/edit/DistortableImage.Edit.js#L473
@rexagod Oh i see, so the point of the fullResolution option is that the user can pass in a full res image and that will be exported instead of the standard one. And this, in some circumstances may have an effect on performance so that is why we provide it as an option?
Exactly. The warpWebGl method could surely cause some performance issues on older systems, therefore we are check its availability by window.hasOwnProperty("warpWebGl").
Note that the warpWebGl method will still render the image even if you've passed it as an option, given your local supports that.
Note that the
warpWebGlmethod will still render the image even if you've passed it as an option, given your local supports that.
What do you mean by this?
Merging this but get back to me here anyways! We can still continue the convo
@rexagod actually wait I take that back. Isn't a little confusing if we name them the same thing? Could we create an example similar to large.jpg so it has a different name, and i notice that large.jpg is a lot larger of a file was this to illustrate the point further
@sashadev-sky What did you mean by,
i notice that large.jpg is a lot larger of a file was this to illustrate the point further
I assumed a rename of a similar image, please do correct me if I'm wrong.
@rexagod oh I thought that it was purposefully intended to be a larger file than the others.
Like I thought that normally, the user wants to use a png file for the best performance, but, if they wanted the highest resolution for their final export, they can pass a jpg version of the file, and that's why the originals are all png and fullRes is jpg in the original example.
Now if we are just using 2 jpg files, which are the same, what are we demoing?
Now if we are just using 2 jpg files, which are the same, what are we demoing?
The difference b/w a webGL rendered, and a normal (un-preprocessed) image, as far as I can tell. We might need to ping Jeff for precise details about the package, though.
@sashadev-sky Where are we on this one? Any changes that you'd want suggest here?
@jywarren could you please advise here about the last few comments when you have a moment? To catch you up quickly we updated the images used in the demos but we forgot to update the full res export image passed as an option. This PR updates the full res export image but now I am wondering about the above questions
Sorry about this - ok, so:
@rexagod oh I thought that it was purposefully intended to be a larger file than the others.
That's right. I used a png and a jpg pretty much without purpose, so they might as well be 2 files called:
-
example.jpg -
large.jpg(orexample-large.jpg)
I do think we should use a pretty good aerial image, for what it's worth. But switching to all-JPG will reduce ambiguity. How does this sound? Thanks!
This is to avoid trying to distort a really big image - like 10mb - when a smaller "preview" image is available.
@jywarren @rexagod should we do an environmentally related picture? Do you have any favorites from the site?
I think once we add multiple images to the demo (if we do) we'd really be able to show the upstream goal of this repo by adding a collection of aerial images that go together to create one scene
@jywarren ok you would probably know better than me here so did you want to select one and get back to me or you want me to find one? Also once we select an image I might just open a new PR and handle this one because there hasn't been any changes here so far to keep and rebasing won't be worth it!
I think this one is pretty cool for index.html
but don't have a link to find it:
@sashadev-sky You can grab that from here. It's a JPEG so no worries, I guess. Also does it have to be about 10mb?
@jywarren ok i'll open a pull to fix this and maybe update the readme to explain fullresolutionsrc, but not sure what constitutes a large image vs a small one? The current large.jpg is only 1.26 MB. The example.jpg is 326.62K . Should we say anything over 1MB users might want to use fullresolutionsrc?
Sure, let's say 'over 500k'? -- Actually 1MB is good or you start to see lag
https://mapknitter.org/maps/tidwell-park
https://s3.amazonaws.com/grassrootsmapping/warpables/306587/IMG_0610.JPG
And smaller version! https://s3.amazonaws.com/grassrootsmapping/warpables/306587/IMG_0610_medium.JPG