Brig/scaling redux #88
Conversation
Calculates new scale level + or - 0.1 based on mouse scroll Clamps mouse zoom min 0.1x, max 4x. Tries to re-center origin, but some more math and bookkeeping needed.
Properly sets dc origin to center. Updates dc origin on canvas resize. Adusts images to draw centered about origin. New ivy-canvas% methods to handle zooming more generically. Change zoom increment to ±0.05. Simplifies all the scrollbar code. General code cleanup and minor refactoring. Known Issues: * Saw centering get messed up for an animated gif (can't repro, may be fixed) * Zoom action buttons haven't been touched.
Make zoom menu coarser. Include 200 and 400% zoom in menu. Switches zoom menu to use new style dc% zooming facilities.
Adds zoom in, zoom out, and reset menu options. Reorganizes zoom menu with separators.
Hooks up zoom action buttons to ivy-canvas dc-based zoom methods. New (zoom-to-fit) method.
New configure-scrollbars canvas method, called on zoom change. TODO: looks like need to adjust image centering for zoomed virtual size? Clean up the old commented-out image scaling code.
Fixes image centering, when zoomed beyond canvas client boundaries. General cleanup and minor code refactoring.
| [insert-tag-tfield-comma insert-tag-tfield-comma] | ||
| [status-bar-position status-bar-position] | ||
| [status-bar-zoom status-bar-zoom] | ||
| [set-fullscreen set-fullscreen] |
There was a problem hiding this comment.
What is the purpose of passing set-fullscreen and toggle-fullscreen as initialization parameters to the canvas? Wouldn't it be more appropriate to define/public them as member functions of the class?
There was a problem hiding this comment.
So if you look at what those functions actually do, you couldn't make them member functions, as-is. The two callbacks you mention, for example operate on the ivy-frame instance, which the ivy-canvas% class doesn't know anything about, per se.
We could call get-parent, but the frame may not always be the direct parent of the canvas. I'd be pretty bummed about a solution where fullscreen stopped working depending on where the canvas was in the layout hierarchy. Perhaps get-top-level-window would work here?
Really, though, the only reason the canvas wants to know about any of this at all is that it's the one intercepting the fullscreen accelerator keys. What would probably be a cleaner solution would be to expose more general on-key callbacks that can be registered from the outside, instead of the canvas knowing/caring about fullscreen stuff at all.
There was a problem hiding this comment.
So short answer is there's a good, technical reason for it, but there's also a couple of alternatives that might be better but will require extra work. Guidance?
There was a problem hiding this comment.
I want to say that having those be member functions is the "better" approach, since get-top-level-window will return the ivy frame, in this case.
Ok, so this is a big one. Lots of cleanup and reorganization. Most notably, it completely purges all the manual image scaling junk, and moves
ivy-canvas%into its own file and makes the whole thing a little more self-contained/modular.Aside from just being fairly major in the changes department, it's probably worth making sure there's no half-done, dangling code.