Extension to keyboard controls
Reference Issue
Fixes #3517
This relates to discussion Keyboard controls https://github.com/google/model-viewer/discussions/3484 In our specific world we are involved with technical drawings, and now with model-viewer, 3D models. The techy nature of it lends itself to wanting to view models at specific views (the orthogonal ones are the obvious examples) and wanting to be able to put models in specific orientations (although most of the users I've observed don't realise that is what they are trying to do with the mouse painfully sliding back and forth over the same spot). It doesn't seem like an obvious need when looking at a model of a show or a tangerine as they don't have the inherent precision that a CAD dwg brings to mind. The current keyboard controls are exactly the right sort of thing, but they are tantalisingly short of functionality.
In working this though there are some code changes in SmoothControl.ts (and a quick one in the package list that is unrealted). These extend the keyboard controls from orbit changes to orbit, pan, step size & preset views and also remove the null hit behavior. The removal of null hit behavior should really be conditional on an exposed state, but I didn't know how to do that. They are contained entirely within SmoothControls.ts.
Keyboard controls are: Arrows - orbit Shift + Arrows - pan Ctrl + Arrows - orbit at small step (ratio from unmodified set as a const, but could ideally be passed in) Ctrl + Shift + Arrows - pan at small steps
Pressing Shift displays the same spot cursor as used on mouse-pan. Shift now behaves as a defacto 'about to pan' indicator. It also accidentally solved a user comment that they didn't really know where it was orbiting around.
Keys 1, 2, 3, 4 (and Numpad 1, 2, 3, 4) set theta to 0, 90, 180, 270 degrees at current phi and radius Keys 7, 8, 9 (and Numpad 7, 8, 9) set phi to +45, 0 and -45 degrees at current theta and radius This gives the ability to 'square up' a view which works in conjunction with the orbit keys.
The ration of small steps to big step should probably be configurable, but as above, that is something that I can't see how to get from the top level interface down to this script.
Only experimental testing has been done. No automated test.
Live Demo Video clips in Keyboard controls https://github.com/google/model-viewer/discussions/3484
The review comments are great. Off the back of them I've rethought everything. There is a another commit to push out with rev2.
Apologies for the commentary responses. I thought I had enough in each of them, but I can see now that I didn't. The new comment against lines 926-929 is probably what I should have asked first before just jumping in and hopefully underpin everything else. The extra diff files are still a total mystery as of this moment.
Just want to double check that I explicitly press Re-request review when I've got to the end of comments (rather than just leaving it)? Thanks.
Just want to double check that I explicitly press Re-request review when I've got to the end of comments (rather than just leaving it)? Thanks.
That would be great, yes. Most people don't bother; I get pinged for commits and comments, so those also tend to get my attention.
As it happens I am waiting on your reply as I'm not sure what to do next and where we got up to. I think I'm in the running for the Mr T blathering fool most ignorant Git user of the project.
Okay, no problem. Please go up to each thread I started in your code and reply to those. Even if the reply is "I'm not sure what you want", that's fine, I just want to see that you've at least read them. We can go from there.
I will do the double check tomorrow when not on a phone, but there are my comments already against each of yours that are waiting for your comments. They date back to the last week in June. Most of them relate to a refractor and drop of my-app specific functions.
I see no replies to any of my comment threads. Is there a chance they're pending and not submitted?
Ok. That was slightly embarrassing. Didn't know that comment visibility (pending/submitted) was linked into the file review actions. I've cost us a month, oops.
It's all good! Live and learn; anyway I've been pretty distracted the last month anyway. Thanks, I'll look over this in more detail tomorrow.
Okay, reading through all of this again, here's what I'd like to do. I want to see between the two of us, how much of what you want can be accomplished externally to MV. Thank you for giving it a shot and reporting your findings, but would you mind pasting that into a Glitch? Then we can work on it together and see the problems live. I have some ideas to work around the problems you found, and if we get it working, I think it could also make a nice addition to our examples.
It's entirely possible we do in fact need MV changes, maybe bug fixes, maybe behavior or API changes, but I want to keep it minimal by checking against an example. Does that sound reasonable?
That is all perfectly reasonable. I've got about a week before a couple of weeks of holidays catch up with the calendar. So step 1 is get something on Glitch, step 2 is work through your other comments. I'm presuming that on Glitch you are looking for things in pairs; The same model in m-v v12 & and then in m-v modified so you can compare and contrast behaviour?
Starter for 10
Using Release m-v https://glitch.com/~iodized-delicious-night
Using Modified https://glitch.com/~tulip-stream-grasshopper
This is a stripped down version of our layout framework. Pls ignore the misaligned frames. The essential elements are buttons within the m-v frame. The Modified version has two lumps of functionality: Key controls for pan and orbit with the inclusion of corse/fine control and the swicth of reframe/select from single to double click.
There are a couple of replies against the files as requested.
Okay, first fix regarding strange zoom-out behavior: Why not just set the max radius and max FoV the same as the initial radius and FoV? I can't see what you were trying to accomplish with 5m and 90deg.
Second, this glitch is really complicated; would you mind removing everything that's not the stuff we're talking about in this issue?
Okay, first fix regarding strange zoom-out behavior: Why not just set the max radius and max FoV the same as the initial radius and FoV? I can't see what you were trying to accomplish with 5m and 90deg.
Because it isn't a truism that the the initial view is the view bounds that I want. The max-es are set to be different precisely so that I can get out beyond the initial view. As an example some of the models are of bits of kit that are ~1.5m tall, 2m long, 1m wide. The initial view is a reasonably tight 3/4 shot, but to view side on or to make a nice orbit you need to be able to pull back further than this initial view.
Second, this glitch is really complicated; would you mind removing everything that's not the stuff we're talking about in this issue?
Big lumps removed, but can take out more if you need me to. It's left with just the side panel and 4 buttons that jump to specific views. These are required (in some shape or other) to force the situation where you need to click back onto the model and hence the view-change part. Keycontrols are independent of this button panel.
Because it isn't a truism that the the initial view is the view bounds that I want. The max-es are set to be different precisely so that I can get out beyond the initial view. As an example some of the models are of bits of kit that are ~1.5m tall, 2m long, 1m wide. The initial view is a reasonably tight 3/4 shot, but to view side on or to make a nice orbit you need to be able to pull back further than this initial view.
Okay, that makes sense. I think because the example given was extreme I was confused. Let's call that a separate bug, which you can file an issue on ideally. Let's fix that on it's own so that click returns to the start instead of the max.
Hi, Sorry for the delay. Stuff going on then lack of WiFi at the holiday destination. I have opened Pan keys 1 #3703 as a new PR to replace this one. Started off small as instructed. Please advise in the new PR. Thanks.
Thanks!