Behavior of enable-pan, max-field-of-view and default field-of-view interaction seems flawed

[rmhall]

Scenario:

Using the latest release of 1.12.0

enable-pan is enabled, camera-target and camera-orbit are both set to "auto auto auto", field-of-view is set to "45deg". Model loads and the user interacts with it, pans the model around using shift-click-drag, and/or zooms in and out using the mouseweel or keyboard changing the field-of-view. They then click off the model, and the model as expected returns back to the default center and the field-of-view is reset to the "45deg" value that was originally set.

If min-field-of-view is used, the behavior remains the same and function as expected, just constraining the minimum field-of-view value for there mouse wheel zoom, and when clicking off the model, center point is returned, and the field-of-view returns back to 45deg.

However, if max-field-of-view is used (by itself or with the min-field-of-view), to permit for larger zoom outs, like 180deg, when a user then clicks off the model, instead of returning the field-of-view back to the initial value of 45deg as specified, the field-of-view is instead set to the max-field-of-view of 180deg. If the max-field-of-view is set to a large value like 90 - this happens in a single click and then it zooms out to 90. If the value is set to an even larger value like 180deg, it sometimes takes 2 clicks, which is also confusing.

What is equally odd, is that in this same scenario even if you have not panned the model yet, and just click outside of it, the field-of-view values are also not reset to the default 45deg (they should be ignored because you should already be at that default FOV), but again when max-field-of-view is set, its used to transform to the field-of-view value instead of the default. This feels like it should be completely ignored if the model is already centered and no longer panned or has never been panned or interacted with yet.

In either scenario, it doesn't seem like the correct behavior and does not correspond to this statement about enable-pan:

"Tapping on the model focuses on that point, while tapping off the model zooms back to the original framing." Which is noted here: https://modelviewer.dev/docs/#entrydocs-stagingandcameras-attributes-enablePan

To me, original framing means the initial field-of-view value (along with other initial values), so it should not be using the max-field-of-view, and instead the specifically set default field-of-view value should be used. The mix/max field-of-view values, should be just that, the outer constraints to control the mouse wheel or programatic control over the model, and should not override the default field of view.

This seems like a bug, or at the least unexpected behavior if that was the intention. If it is intentional, I'd like to understand the logic and also is there an easy way to override this behavior to still allow panning, but to override the results of the "click off the model" trigger, so that it will either honor the original field-of-view value and ignore the max-field-of-view value, or is there an event for that click we can subscribe to and override directly? Or could there be an additional property to define the specific FOV value that should be applied when clicking off the model if enable-pan is used? It truly seems like this is unexpected behavior currently.

I would settle for the expected behavior to be that it honors the default field-of-view value and ignores the max-field-of-view, but it might be even more valuable to have an event and a way to override that "click off the model" that resets the pan center/position and camera orbit.

[rmhall]

Digging in a little bit - I think I see the culprit, in the private function of "recenter" in the /src/three-components/SmoothControls.ts

I see this starting at line 630 of the current master branch:

if (hit == null) {
  const {cameraTarget} = scene.element;
  scene.element.cameraTarget = '';
  scene.element.cameraTarget = cameraTarget;
  // Zoom all the way out.
  this.userAdjustOrbit(0, 0, 1);

It's that last line of userAdjustOrbit 0, 0, 1

Which in my opinion, it should again not be going all the way out to the max, it should be going back to the initial value, or at the very least, allow that value to be overridden or defined more explicitly.

It just feels like this is not fully baked, or at least not well defined behavior that is not intuitive. Am I off base with this assessment?

[elalish]

Well deduced! Would you mind making a PR to fix this?

[rmhall]

Thanks @elalish - glad I was not too off base on this.

So my question back for you, before putting together a PR is, how best do you think this should be solved? And what was the original intention here?

I ask because I confirmed by making a quick build where I removed that last line calling the userAdjustOrbit or by changing the last argument to a zero prevents the erroneous zooming out. That is great for me and my needs, but I wasn't clear on the original intention there. I don't neccesssarily want to wade into changing the behavior directly of the adjustOrbit that gets called by the private userAdjustOrbit, as I can see this actually being more of a preference, and thus maybe more involved?

By that I mean, I can see how a developer/user might want control over this more granularly. Setting aside the issue where it zooms all the way out as is to the max FOV setting, should there even be an attempt at adjusting the orbit here when simply clicking off the model to recenter it? Perhaps there is a need for that? If so I am unclear as to why, but maybe I just don't have a good use case. If there are use cases and a need, maybe there should be some additional properties/parameters exposed to tailor that behavior more directly? For example/discussion sake:

pan-recenter-fov-zoom-orbit: with the options: "none", "auto", "min", "max", percentage, FOV in degrees

Where those argument/options equate to:
none = no zoom, do not change orbit or FOV when re-centering
auto = zoom to FOV specified by the original field-of-view setting/default
min = zoom to FOV specified by min-field-of-view setting/default
max = zoom to FOV specified by max-field-of-view setting/default
percent = relative percentage (if that even makes sense in this context)
FOV = specific FOV in degrees

Does it need to be this granular to the nth degree, or would simply removing the adjustment of the orbit/zoom via the call to this.userAdjustOrbit(0, 0, 1); line be sufficient? I am unclear as to the original intent as to why that is there or if its really necessary to also change the fov/zoom/orbit at all when simply re-centering after a pan operation. My logic is that it is not needed or desirable, but again I may not have all the use cases thought out.

It may be as simple as just removing it (at least for me it is), but it feels like maybe there was more to this originally?

[elalish]

The intention was to return to the initial zoom, rather than the max. If you're very zoomed-in, just re-centering can be unclear, as you lack the context to see that you're centered. The idea is to give the user an easy way back to where they started, but without changing their view angle. I'd prefer to avoid adding another API for this unless I get a lot of community interest. Does that sound reasonable?

[rmhall]

Ahh got it - yes I didn't think of that use case, or if you were really zoomed way out too... That makes complete sense and reduces the complexity. Yes, totally reasonable, and I only laid out all those other granular options if the intention or need was maybe much different than what I was thinking. I'll spend some time reviewing the best way to ensure it goes back to the original FOV/orbit/zoom instead of the MAX and once I've got a handle I can put a PR together - thanks!

[sameeryel]

@rmhall Is there a way to fix this issue by ourselves or should we wait for an official update? I'm using this for a personal project and the interaction with the pan enabled makes it very confusing for the user experience. The simplest way would be to do nothing if the user clicks outside, even that is better than the 3D model jumping around.

[elalish]

I've seen enough feedback like this I think I'll just add an attribute, say disable-tap, so this can be turned on and off. Though any UX feedback on better behaviors for it are also much appreciated.

[elalish]

See #3611

[sameeryel]

Excuse my knowledge. Is it ready to use? or will I have to wait till its implemented?

[elalish]

lol, I said I planned to implement it 3 minutes ago. I appreciate how quickly you think I work, but it will take a bit more time...

You can always make the PR yourself if you want it faster. 😄

[sameeryel]

I'm sorry I'm new to the whole github system and even whole world of coding. Navigating my way through one line of code at a time.