search

NVlabs / nvdiffrast

Nvdiffrast - Modular Primitives for High-Performance Differentiable Rendering
Other
1.29k stars 139 forks source link

Add `dr.texture(max_mip_level="MAX")` #140

Closed JulianKnodt closed 7 months ago

JulianKnodt commented 8 months ago

Currently, NVDiffrast supports power of two values when max_mip_level=None, otherwise it is expected that a max mip level is passed. For my use case, I have a number of images which are not powers of two, and need to optimize textures for all of them, but they all have different sizes. Before each one, I manually compute the maximum possible mip level, by repeatedly dividing by 2 as long as it's even, which is a bit of a pain.

The documentation reasons that it shouldn't automatically have this behavior, which I agree with since it would randomly lead to quality degradation for images of different sizes.

That being said, I think it could make sense to add this in is as behavior that could be opted into, if the user knows what they are doing. Akin to Eigen's EIGEN_YES_I_KNOW_SPARSE_MODULE_IS_NOT_STABLE_YET preprocessor directive, maybe a name like MAX_SUBOPTIMAL_MIP could be used (altho I personally think MAX) would be fine.

s-laine commented 8 months ago

I feel like this would be better addressed by a wrapper function rather than a modification to nvdiffrast itself. Keeping the interface minimal has been one of the goals of the project, so building your own abstractions on top of nvdiffrast primitives is encouraged.

JulianKnodt commented 8 months ago

That's a fair point, nvdiffrast's simplicity is quite nice. Subjectively, I think adding a flag wouldn't complicate the API, but that's not for me to decide.

I'm not sure how common/uncommon using a non-power of two is, as academic work that uses this generally just target 512 or 1024, but for use in other applications I feel like it might be much more common, and if it is common enough maybe it's worth adding?