natashawylie / iviewer

image viewer plugin for jquery
233 stars 97 forks source link

Implement new 'frame' parameter in 'info' method #45

Open oaubert opened 11 years ago

oaubert commented 11 years ago

It returns values (in percentage) for displayed area of the image. This allows to implement the drawing of the displayed area in a thumbnail for instance.

can3p commented 11 years ago

Did you get into account the rotation? For example, see how is orig_height and orig_width are implemented in the info method. The second thing is that it seems that this method is not enough to draw thumbnail window, because you need to update it after every change of image zoom or position. The last question is - why are you passing zoom value here? You can already retrieve zoom from info method.

oaubert commented 11 years ago

Indeed, I did not take rotation into account. It seems an exotic use case for me anyway, but I guess it could be addressed by adding the angle information to the returned structure, and letting the user process it the way he wants.

For the update question, I currently invoke it in a method that is bound to the onAfterZoom and onStopDrag events, because I do not want continuous update. It could also be used in methods bound to onZoom/onDrag for continuous updates. Of course, another approach could be to implement it as a new event (or maybe 2: onVisibleFrameUpdate for continuous updates and onAfterVisibleFrameUpdate for non-continuous updates), but I first wanted to implement the feature with minimal changes.

Lastly, for the zoom information, I do not use it for the moment but I guess it could be a potentially useful information in this context, and the cost of putting it into this structure is far less than an additionnal call to the info method.

oaubert commented 11 years ago

I have just added an example file so that you can experiment with this new feature.

can3p commented 11 years ago

Can you update the code to handle the rotation properly? I think that thumbnailer is a great feature the can fit nicely into the widget core, however it should react on all properly in all corner cases. The reason why this should be implemented is uniformity of the API. All the public methods switch width and height depending on the angle, so that user can avoid implementing this logic by himself.

oaubert commented 11 years ago

Honestly, not seeing a valid use case for this kind of setup (a 90 degree rotated image with a non-rotated thumbnail?), I cannot properly define, hence implement, rotation handling. Do you have concrete examples of rotation uses to make things clearer?

can3p commented 11 years ago

No, it's the thing I'm trying to avoid. E.g. if in non rotated state the function return { x: 0.1, y: 0.2, w: 0.3, h: 0.4 } then the values cannot be the same if we rotate the image 90 degrees. Later this day I will try to fix it in code and push it to your fork, so all the changes could be in one place.

Silex commented 10 years ago

Ah, this is very nice, I was looking about how to implement a "minimap" for the current image, and this seems to solve it. Any chance to get this merged soon or should I use @oaubert fork?

can3p commented 10 years ago

@Silex we still need to achieve a consistent behaviour in cases when image is rotated. Could you look into this? If we fix this corner case I'll merge this in the master

Silex commented 10 years ago

I'll look into it... but I'm not sure it make sense to have a minimap for rotated images. I think it makes more sense for the minimap to be rotated as well?

can3p commented 10 years ago

Yes, sure. I think that minimap should look the same as main image. So if our image is rotated then it makes no sense to have the minimap that is not rotated