aclysma / rafx

Multi-backend renderer with asset pipeline. The objective of this repo is to build a scalable, flexible, data driven renderer.
Apache License 2.0
641 stars 32 forks source link

Confusing `rafx_visibility` and `rafx_framework::visiblity` exported as `rafx::visibility` #166

Closed zicklag closed 3 years ago

zicklag commented 3 years ago

I was just going through the crate docs and noticed a rather confusing module set, where the rafx_visibility crate is re-exported as rafx::rafx_visibility, but the rafx_framework::visibility module is re-exported as rafx::visibility. I'm not sure yet what the differences between the pieces are so I can't suggest any less confusing way to set it up yet, but once I get into it I might submit a PR for a different organization.

aclysma commented 3 years ago

The history of it is that there used to be a rafx_visibility crate, then I pulled it into the rafx_framework crate to reduce modules, and then @dvd added the crate back when he implemented an actual visibility algorithm.

@dvd do you have any thoughts or ideas for how we could name this. I personally don't feel we should bother changing this unless we have an idea we're pretty happy with. Long-term, both these crates/modules will be internal details that few people will ever touch.

zicklag commented 3 years ago

If this is all in flux and going to change later anyway, I'm not opposed to leaving it how it is. I'm always pushing towards very tidy and easy-to-understand documentation and organization, but I totally get it if this is all work-in-progress and it's not worth spending time on this yet.

aclysma commented 3 years ago

Going to close for the time being, I want to delay final decisions for naming/crate layout until later on when we've been able to do more road-testing of these systems.