search

raysect / source

The main source repository for the Raysect project.
http://www.raysect.org
BSD 3-Clause "New" or "Revised" License
88 stars 23 forks source link

Wrong construction of PlanoConvex lens #363

Closed Mateasek closed 4 years ago

Mateasek commented 4 years ago

PlanoConvex lens is constructed in a wrong way. The barrel is too short to form a lens in intersection. I can quickly push a fix, but I would be happier if somebody could look at the PR #355 which also fixes this problem.

CnlPepper commented 4 years ago

I think that would only be true for very small curvature radius, long barrel lenses. I'll have a look at the PR when I get a spare moment. The solution would be to union a cylinder of the required length before the intersection.

Mateasek commented 4 years ago

I discovered it when I was trying to benchmark model of one of our objectives, so it is definitely happening for realistic lenses.

CnlPepper commented 4 years ago

Ah the joys of being the first person to fully use a feature, you get to find all the real world bugs.

CnlPepper commented 4 years ago

Apologies for not tackling your pull requests, most of the items you have been working on require me to sit down and think carefully. Unfortunately I'm only got the capacity for minor quick fixes at the moment, or mindless restructuring work. I will get to these and I am grateful for them. The new company is drawing most of my cognitive time. It should get easier over the coming months.

mattngc commented 4 years ago

Hey Alex, I'd discussed with Matej yesterday that I'd probably handle that pull request as its a part of the code I put together and its non-core. But I'm happy to hand it over to you if you want to look at it.

Mateasek commented 4 years ago

@CnlPepper there is nothing to apologise for. I understand that you have to be busy with other more important work and also that accepting a PR has to be demanding. I was reacting to your assessment of the lens type the bug can happen to. I can push a short fix for the lens which should require less time to review and the #355 can still wait (it fixes more exotic lenses).