Skip to content

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Dec 13, 2013

Sorts translucent objects back-to-front for correct ordering. Uses the distance from the command bounding sphere to the camera. We might want to update the other bounding volumes so there can be tighter bounding volumes for commands.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use an @example.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 14, 2013

Update CHANGES.md. No need to mention the code changes deep in the renderer, but we should mention that we improved the visual quality of overlapping translucent objects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid DOC_TBA in new code.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 14, 2013

Tests and examples are good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comparison function used with Array.sort is supposed to return (negative/zero/positive) for a (less/equal/greater) b, so I think you want to subtract the distances here.

@bagnell
Copy link
Contributor Author

bagnell commented Dec 17, 2013

@pjcozzi This is ready for another review.

That last commit feels like a hack to me. Something needed to be added to the commands so they weren't sorted. Adding an index made it so that it worked for CompositePrimitives that contained other CompositePrimitives. If you have better ideas, I'm open for suggestions. Perhaps we need to add a separate primitive, OrderedCompositePrimitive?

Conflicts:
	CHANGES.md
@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 21, 2013

Let me know when this is ready for another review. I can help design the OrderedCompositePrimitive but I'd rather just see what you come up with.

Conflicts:
	Source/Core/BoundingSphere.js
	Specs/Core/BoundingSphereSpec.js
Conflicts:
	Source/Renderer/Pass.js
	Source/Scene/Scene.js
@bagnell bagnell mentioned this pull request Jan 2, 2014
@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 10, 2014

Should we close this for now?

@bagnell bagnell closed this Jan 10, 2014
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.

3 participants