Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider simplifying input/camera approach #522

Closed
hannobraun opened this issue May 4, 2022 · 6 comments
Closed

Consider simplifying input/camera approach #522

hannobraun opened this issue May 4, 2022 · 6 comments
Labels
help wanted Extra attention is needed topic: display Displaying Fornjot models type: planning Issues about project planning

Comments

@hannobraun
Copy link
Owner

A few months ago, before I started working on Fornjot full-time, I was working on Fornjot's input/camera code, trying to make it more usable than what I've seen in other CAD applications and similar programs. The advanced input/camera system I built includes the following features:

  • You can accelerate the zooming speed by continually turning the mouse wheel, and stop an ongoing zoom by turning it into the other direction. The intent here is to allow for both quick movements (to quickly zoom out to get an overview, for example) and fine control while zoomed in closely.
  • When moving the model (move mouse while holding right-click), the speed of the movement is dependent on where the mouse pointer is, so the model doesn't move quicker or slower than the mouse moves.
  • When rotating the model, it rotates around the point, where the mouse pointer touches the model (called the focus point). The intent here is to allow for close inspection of a feature while zoomed in.

Unfortunately, this never worked very well:

These problems are still kind of acceptable, as long as Fornjot is still as experimental as it is. But it won't stay acceptable for much longer. There are basically two options:

  • Revert to simple and predictable behavior.
  • Fix and improve the current behavior.

Or any combination of the two.

I'm labeling this issue as https://github.com/hannobraun/Fornjot/labels/help%20wanted. I don't think I'll have the capacity to focus on this any time soon, so if we are to keep the current non-trivial behavior, someone will have to step up. If no one does, I plan to revert to simple behavior, close all the relevant issues, and add an item about more advanced input to the feature wishlist.

@ObiWanRohan, you expressed interest in working on input/camera things. Are you still planning to do that?

@hannobraun hannobraun added help wanted Extra attention is needed topic: display Displaying Fornjot models topic: input type: planning Issues about project planning labels May 4, 2022
@hannobraun hannobraun added this to the Basic Usability milestone May 4, 2022
@chrisprice
Copy link
Contributor

I've had a quick dig into this and I'm keen to keep digging.

So far I have a very primitive version of #17 which just draws a triangle using the first 2 points from the closest triangle and the cast cursor position. It really isn't very exciting to look at but here's what happens if I try and click in the middle of a triangle (fj::Sketch::from_points(vec![[-1., 0.], [0., 1.], [1., 0.]]).into() -

image

However, to make the focus point appear at the top, I need to click lower than expected. The same error happens at each vertex and increases as you move away from the origin. I can make the error worse by increasing the aspect ratio of the window and correct it by ensuring the aspect ratio is 1.

I haven't yet found the root cause of this but I expect it is contributing to #18.

@hannobraun
Copy link
Owner Author

Interesting, @chrisprice, thanks for looking into it!

@chrisprice
Copy link
Contributor

After some more experimentation I noticed that changing the screen dimensions, changed the relative horizontal size of the triangle. After some more digging, this is the suspicious code -

    pub fn for_vertices(camera: &Camera, aspect_ratio: f64) -> Self {
        let field_of_view_in_y = camera.field_of_view_in_x() / aspect_ratio;

        let transform = camera.camera_to_model().project_to_array(
            aspect_ratio,
            field_of_view_in_y,
            camera.near_plane(),
            camera.far_plane(),
        );

        Self(transform.map(|scalar| scalar.into_f32()))
    }

Occam's razor suggests it's unlikely to be an issue in another library, so that pretty much leaves the x/y field of view conversion. It turns out it is an issue in another library, just not one we're using... Here is the same issue from Three.js (so we're in good company!).

Here's the suggested conversion from that issue -

xfov=2*Math.atan(Math.tan(yfov/2) * aspect)

Converting to Rust and inverting the calculation -

field_of_view_in_y = 2. * ((camera.field_of_view_in_x() / 2.).tan() / aspect_ratio).atan()

I've run out of time for now but will create a PR soon.

@hannobraun
Copy link
Owner Author

Thank you for the investigation, @chrisprice! If I remember correctly, I wrote this code pretty much from scratch, so if there were basic mistakes to be made, I'm not surprised I made them.

I'll take a look at your PR later.

@hannobraun
Copy link
Owner Author

Looks like the issue @chrisprice found, while a legitimate bug, isn't related to input/camera stuff. See my review of #614 for context.

@hannobraun
Copy link
Owner Author

Thanks to the awesome work done by @chrisprice (thank you!), input works much better now, and reverting to the simple behavior is no longer a consideration for me. I have therefore decided to close this issue.

This issue mentions a few problems for which no issues existed before. I have created those now:

There are other input issues, and all of them are labeled as https://github.com/hannobraun/Fornjot/labels/topic%3A%20input.

All of those issues should be relatively easy to fix, at least compared to what @chrisprice has already done.

Thank you for your work on this, @chrisprice! I sometimes get quite annoyed when using OpenSCAD, for example when I'm zoomed in very closely and need to adjust the direction I'm looking at the model from a tiny bit, but then the model completely disappears from my view, because it rotates around some far-away point. The more advanced input model of Fornjot was meant to address problems like that, but it was always a bit too ambitious, given the limited effort I was able to put into it.

You changed all that and just made it work (barring the few issues that are still open), and I really appreciate that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed topic: display Displaying Fornjot models type: planning Issues about project planning
Projects
None yet
Development

No branches or pull requests

2 participants