Foreshorten to the true perspective eye at view_distance / strength - #87
Merged
Conversation
A Perspective(strength=s, view_distance=D) places the pinhole eye at D/s, but silhouette radii and the bond-cap eye distance used D directly, so the perspective bulge never converged to the orthographic radius as strength fell. Correct both together, since atom silhouettes and the bond caps meeting them must share one eye: - silhouette_radius: the exact silhouette for an eye at D/s carries (r*s)^2, not r^2. Computed as sqrt(d - rs) * sqrt(d + rs) so the denominator stays finite for large view_distance, where the squared form d^2 - (rs)^2 overflows float64 (past ~1.3e154) and zeroes every radius. The denominator floor of 1e-6 is the square root of the old 1e-12 floor on d^2 - r^2, so an eye-inside-sphere still yields a huge finite radius rather than a division by zero, and now also warns. - eye_distance: returns view_distance / strength.
Harden the tests around the perspective silhouette change: - Pin a hand-computed silhouette radius at strength 0.5, where the corrected (r*s)^2 term diverges from the old r^2 form; the existing hand-computed assertion was at strength 1, where the two coincide. - Assert the eye-inside-sphere case yields a huge but finite radius, so the 1e-6 denominator floor is guarded rather than only its warning. - Add a dedup test for the sphere-contains-eye warning, matching the eye-plane one. Also correct the _sqrt_difference_of_squares docstring: the naive product overflows to inf, and it is the caller's division by that inf that drives the silhouette to zero.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
Perspective(strength=s, view_distance=D)places the pinhole eye atD / s, but two computations usedDdirectly, so the perspective bulge never converged to the orthographic size as the strength fell. This corrects both, coupled in one commit because atom silhouettes and the bond caps meeting them must share one eye.(r * s)^2rather thanr^2, measured from the eye atD / s. The denominator is factored assqrt(d - rs) * sqrt(d + rs)so it stays finite for largeview_distance, where the squared formd^2 - (rs)^2overflows float64 and zeroes every radius. A sphere large enough to contain the eye now warns instead of ballooning silently.eye_distance = D / s), so a cap and the atom it meets stay consistent at strengths other than one.assert isinstancedowncast that only existed to interpolate the parameters.Unlike the two preceding projection PRs this changes rendered output on purpose, and only under perspective — orthographic output is unchanged. At
strength == 1both corrections are no-ops ((r*1)^2 == r^2,D/1 == D).Out of scope, each to follow separately: viewport magnification sizing, the
look_alongconvention cleanup, and oblique projection.