Fix x/y field of view conversion

[chrisprice]

See #522 (comment)

[hannobraun]

Thank you for this pull request, @chrisprice!

I can confirm that this pull request fixes a bug, however, it doesn't improve any of the input/camera problems that #522 tracks.

Here's what I found:

• graphics::Transform::for_vertices only has one caller, Renderer::draw. This is purely graphics output code, so it can't have any influence on the input and camera control code.
• And indeed, the focus point calculation problems (Computation of focus point is not fully correct #18) are still there. This can be easily confirmed by moving a model around, then try to rotate it. Once you've moved and rotated it for a bit, it will almost never rotate around the point where the mouse pointer points.

So I wondered, since you didn't include the debug drawing code you mentioned in #522, is there some way I can observe the change made in this PR? Because I didn't really notice a difference under regular circumstances.

Playing around a bit, I noticed a bug that I was previously unaware of: If you make the window narrower (i.e. small in the x direction), the model gets smaller to the point of disappearing, then large to the point of no longer fitting the window, then smaller again. This keeps repeating.

This change fixes that bug: If you make the window narrower, the model just keeps getting smaller, as expected.

So yeah, this is a welcome fix, and I'm definitely merging it. But it seems the bug this fixes has no relation to #18/#522.

[chrisprice]

So I wondered, since you didn't include the debug drawing code you mentioned in #522, is there some way I can observe the change made in this PR? Because I didn't really notice a difference under regular circumstances.

Here's a work in progress version which shows the focus point.

And indeed, the focus point calculation problems (#18) are still there. This can be easily confirmed by moving a model around, then try to rotate it. Once you've moved and rotated it for a bit, it will almost never rotate around the point where the mouse pointer points.

My hunch is that #18 describes at least 2 problems. Quoting from #148 -

This works mostly, but not fully. The focus point is not detected correctly when the cursor still points at the model, but just barely. Maybe the graphics and the focus point computations use slightly different perspectives, leading to some distortion. This also seems dependent on window size (or at least aspect ratio), as the effect is less pronounced when the window is square.

I believe this is the problem I've fixed.

I'd like to add to that that rotation seems a bit weird. It seems to me like it doesn't rotate around the focus point, but a point somewhat offset from that.

I believe this is the problem you're still seeing.

Apologies for the lack of clarity. I'm still trying to learn my way around the codebase. I pushed this PR up on its own as it seemed like an easy and obvious standalone fix.

On reflection I think I've added to the confusion by referencing the PR in #522 instead of explicitly referencing the sub-issue of #18. My goal in referring to #522 was just to show my interest in ultimately tackling that issue. I'll try to be more explicit in future.

[hannobraun]

Here's a work in progress version which shows the focus point.

Nice! Looking forward to seeing that as a PR 😄

This works mostly, but not fully. The focus point is not detected correctly when the cursor still points at the model, but just barely. Maybe the graphics and the focus point computations use slightly different perspectives, leading to some distortion. This also seems dependent on window size (or at least aspect ratio), as the effect is less pronounced when the window is square.

I believe this is the problem I've fixed.

Oh yeah, I totally forgot about that. Even though that's what #18 was about originally, only the "not rotating around focus point" problem stuck in my mind.

And indeed, I can no longer see any deviation here. Moving the model works, if the cursor is exactly over the model, and doesn't work if it's not (as intended).

I'd like to add to that that rotation seems a bit weird. It seems to me like it doesn't rotate around the focus point, but a point somewhat offset from that.

I believe this is the problem you're still seeing.

Yes, I agree.

Apologies for the lack of clarity. I'm still trying to learn my way around the codebase. I pushed this PR up on its own as it seemed like an easy and obvious standalone fix.

Any incremental improvement is welcome! I just wanted to provide as much context as I could, for your benefit, and the benefit of anyone who's going to read this later (including myself). This wasn't meant as criticism for not fixing the problem completely, or anything like that.

On reflection I think I've added to the confusion by referencing the PR in #522 instead of explicitly referencing the sub-issue of #18. My goal in referring to #522 was just to show my interest in ultimately tackling that issue. I'll try to be more explicit in future.

Well, as you noted, #18 referred to 2 different problems in the first place, so if there was any confusion, that was the source.