Incorrect construction of BiConcave lens?

[Mateasek]

I think I've spotted a problem in the construction of the back surface of the BiConcave lens. I tried to verify it with this simple test, where I defined edge points which I believe should be containted by the lens and then tested it by lens.contains.

import unittest
from raysect.core import translate, Point3D
from raysect.primitive.lens.spherical import BiConcave
from raysect.optical import World

class TestSphericalLense(unittest.TestCase):

    def test_biconcave(self):

        world = World()
        diameter = 75
        center_thickness = 5
        front_curvature = 180
        back_curvature = 200

        lens = BiConcave(diameter, center_thickness, front_curvature, back_curvature, parent=world)

        pad_constant = 1e-4

        padded_radius = lens.diameter / 2 - pad_constant

        # key points on the back lens surface
        points_back_surface = {}
        padded_z_back = -1 * (lens.back_thickness - pad_constant)  # z component of lens back surface edge
        points_back_surface["centre"] = Point3D(0, 0, pad_constant)
        points_back_surface["x_edge_pos"] = Point3D(padded_radius, 0, padded_z_back)
        points_back_surface["x_edge_neg"] = Point3D(-padded_radius, 0, padded_z_back)
        points_back_surface["y_edge_pos"] = Point3D(0, padded_radius, padded_z_back)
        points_back_surface["y_edge_neg"] = Point3D(0, -padded_radius, padded_z_back)

        # key points on the front lens surface
        points_front_surface = {}
        padded_z_front = lens.center_thickness + lens.front_thickness - pad_constant # z component of lens front surface edge
        points_front_surface["centre"] = Point3D(0, 0, pad_constant)
        points_front_surface["x_edge_pos"] = Point3D(padded_radius, 0, padded_z_front)
        points_front_surface["x_edge_neg"] = Point3D(-padded_radius, 0, padded_z_front)
        points_front_surface["y_edge_pos"] = Point3D(0, padded_radius, padded_z_front)
        points_front_surface["y_edge_neg"] = Point3D(0, -padded_radius, padded_z_front)

        for key, item in points_back_surface.items():
            self.assertTrue(lens.contains(item), msg="Back surface incorrectly constructed.")

        for key, item in points_front_surface.items():
            self.assertTrue(lens.contains(item), msg="Front surface incorrectly constructed.")

The back surface points except the ceter one are not contained by the lens. I believe this is cause dy wrong shift of the lens barrel during initialisation:

        padding = self.edge_thickness * PAD_FACTOR

        # construct lens using CSG
        front = Sphere(front_curvature, transform=translate(0, 0, center_thickness + front_curvature))
        back = Sphere(back_curvature, transform=translate(0, 0, -back_curvature))
        barrel = Cylinder(0.5 * diameter, self.edge_thickness + padding, transform=translate(0, 0, -0.5 * padding + self.back_thickness))
        lens = Subtract(Subtract(barrel, front), back)

I believe the problem is connected to the translation of the barrel having wrong z component value.

Also I would like to ask how the length of the barrel influences the performance during tracing? If it does not matter, than simply stretching it in between centres of the curvatures would be robust solution. If it does matter, then it could be made almost half of what it is now. I could review rest of the spherical lenses and make simmilar tests for all of them.

[CnlPepper]

I can confirm there is a bug. You can see it in the lens render:

Looking forward to these unit tests! Never had the time to write them for the less well used area of the framework.

[CnlPepper]

Looks like it was due to a typo. The barrel was shifted forward, not backwards. It should be correct now.

[Mateasek]

Great, thanks for the fix. I will make tests for all the lenses and make PR.

[CnlPepper]

Excellent, thanks.