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

Introduce cone primitive #83

Merged
merged 4 commits into from
May 3, 2020
Merged

Introduce cone primitive #83

merged 4 commits into from
May 3, 2020

Conversation

w0rm
Copy link
Collaborator

@w0rm w0rm commented May 1, 2020

This is work in progress, for now I'm rendering Cylinder3d as a cone because it is difficult to integrate with local elm-geometry.

The Cone3d type is introduced in this pr.

After changing Demo.elm to render the cone, I got a weird shadow on the cone, that looks different for the cylinder.

Screenshot 2020-05-01 at 17 39 25

Screenshot 2020-05-01 at 17 46 10

@ianmackenzie
Copy link
Owner

Ah OK I think I know what's going on and it's my fault - currently elm-3d-scene does not have proper handling of normals (multiplying by the inverse transpose of the model matrix) when non-uniform scaling is involved. I knew this was a potential issue but it's been fine up until now since it's not possible to apply non-uniform scaling to general meshes; the only other place that's actually used currently is for blocks, and there it's OK because the non-uniform scaling will always be axis-aligned so doesn't actually affect normals at all. But it doesn't work for cones! (Things do seem to render properly if you use a cone where length is equal to radius, so that there's no non-uniform scaling involved.)

For cones we will have to add some sort of proper normal transformation code. I'll probably put in something fairly specialized right now so we can get cones working without introducing the overhead of computing inverse transpose matrices all over the place, but ultimately I would like to support non-uniform mesh scaling (#54) and so we will need to figure out an efficient general-purpose solution.

@ianmackenzie
Copy link
Owner

As a separate point, note that Primitives.cylinder has the base point of the cone as the origin point, but then Entity.cone takes that mesh and calls placeIn centerFrame - so I think the placement of the cone will end up being offset by half its length.

@w0rm
Copy link
Collaborator Author

w0rm commented May 1, 2020

Yeah, the origin of a cone is in the bottom, that is why it is already hanging in the air.

        centerFrame =
             Frame3d.fromZAxis (Cylinder3d.axis givenCone)

Once replaced with Cone3d.axis will be aligned to the bottom of the cone shape.

@ianmackenzie
Copy link
Owner

Yup, figured that issue would just go away once the proper cone shape is in, but felt I should point it out anyways just in case you were wondering yourself what was going on 🙂

I'll take some time this weekend to figure out a solution to the non-uniform scaling of normals problem.

ianmackenzie pushed a commit that referenced this pull request May 2, 2020
Will be necessary for rendering cones (#83)
@w0rm
Copy link
Collaborator Author

w0rm commented May 3, 2020

@ianmackenzie I think this is ready now!
Screenshot 2020-05-03 at 18 07 13

@ianmackenzie
Copy link
Owner

Made a couple very minor tweaks, but I think this looks good!

@ianmackenzie ianmackenzie merged commit f7c251b into ianmackenzie:master May 3, 2020
@w0rm w0rm deleted the cone3d branch May 3, 2020 17:43
@ianmackenzie ianmackenzie mentioned this pull request May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants