Skip to content
This repository has been archived by the owner on Dec 29, 2020. It is now read-only.

Scaling algorithm to fit images to slider frame #10

Closed
wants to merge 86 commits into from

Conversation

fixermark
Copy link

Greetings!

I've been using slider.js for a personal photo spinner project. I thought people might be able to use the modifications I made to support images of heterogeneous sizing; in the original configuration, an image's width is fit to the width of the slider and the height is scaled to maintain aspect ratio (but the bottom of the image might be clipped).

This modification updates the image size relative to the slider frame so that the entire image fits in the frame. More specifically, the image is scaled up or down as needed so that:

  • the entire image fits in the frame
  • aspect ratio is maintained
  • one of the dimensions (width or height, depending on the aspect ratio of the frame and the image) is filled.

-- Known Issues --

  • Resizing the slider does not recompute the image sizes.
  • slider.min.js has not been updated and the docs have not been updated (I lack a functional install of yuicompressor or doccos).

Let me know if you have any questions!

Sincerely,
Mark Tomczak
Pittsburgh, PA, USA

gre and others added 17 commits October 28, 2011 21:34
TODO:
* Current implementation visually assumes that the image takes up the full
  width * height for all images. May need to e.g. render image to black frame,
  do transition between black-framed image and next image.
* Current implementation fits width for CSS-transition images. Want to rescale
  in CSS (maybe per-image?) so that each image fits.
…: Test with images smaller than frame; test canvas mode.
@gre
Copy link
Owner

gre commented Nov 25, 2011

Thanks a lot for this man!

I will look at this very soon,

Best regards,
Gaëtan Renaudeau

@gre
Copy link
Owner

gre commented Nov 27, 2011

3 notes:

  • Canvas transitions don't work for me (on the demo page)
  • the photos.json mentions a images/chocolate.jpg I don't have
  • I don't really like the _sizeAllImgs, I'm wondering if there is a way to do so with more CSS. Maybe we can't and you right. But I would prefer this code to be done in the tmpl method because it allows more customizability (I plan a setTemplate method soon).

But again, I like your improvment, It will be part of the v1.2 release with some refactoring.

@fixermark
Copy link
Author

SGTM. Thank you for taking a look at this request!

I was seeing the canvas issue too, but I assumed it was a speed issue with my development machine (it's pretty slow; canvas transitions had seemed to never work). I'll load onto a more powerful machine and take a look into that issue to verify that I haven't introduced a bug.

images/chocolate.png was an image with an aspect ratio where height exceeded width, which I was using as a test image. I have terrible taste in art, so if you'd like to either substitute an image or drop that file reference, feel free.

@fixermark
Copy link
Author

Have you gotten any additional insight regarding the canvas transitions issue? I was able to fixermark/slider.js and gre/slider.js alongside each other on my faster machine (using Chrome 16.0.912.41 beta-m), and I wasn't able to notice any significant performance difference between the two.

Oh, wait... Are you referring to the bizarre "pop" effect you get when an image of a different size is switched to by a canvas transition? My apologies; I meant to list that as a known issue. Not quite sure yet what the right way is to fix that; I think the underlying cause is that the canvas doesn't do any drawing outside of the image bounds (this isn't an issue when the images are the same size; when images are different sizes, we're left with holes). I'll see if I can come up with a fix for that issue.

@fixermark
Copy link
Author

Update: I think I've addressed note 1 and 2 with this set of commits. You may want to add a photo with a different aspect ratio to photos.json to demonstrate the new sizing feature; I'd add chocolate.jpg, but it's a goofy vacation photo. :)

@gre
Copy link
Owner

gre commented Dec 3, 2011

Thanks for this commits, no problem with different size images, we can test with Flickr ;)

I'll test this soon ;)

@euler0
Copy link

euler0 commented Feb 29, 2012

Has there been any progress on this issue? ^-^

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants