davvid
commented
7 months ago I wasn't sure if that was perhaps something we should do in this crate in ptextexture_get_bordermode{u,v} and ptextexture_get_edge_filter_mode, and if so what the defaults should be: I would suppose EdgeFilterMode::None, and BorderMode::Black?
I like the idea of us handling it in the crate and defaulting them to EdgeFilterMode::None and BorderMode::Black like you suggested.
We are technically poking at bits so I'm not sure if there's much ptex can do. There are couple of functions where ptex does check for out of range values and handles them:
There are a couple of case statements that fully enumerate the known-to-c++ enum values, e.g.:
- https://github.com/wdas/ptex/blob/ea6890e041c3889ff7cb80a8546d2a3b6b8804bf/src/ptex/PtexSeparableFilter.cpp#L77
- https://github.com/wdas/ptex/blob/ea6890e041c3889ff7cb80a8546d2a3b6b8804bf/src/ptex/PtexSeparableFilter.cpp#L83
These case statements seem like the only parts that might need to change in upstream ptex. If we did that, would adding default: to them above the m_black case be all that's needed?
Arguably, default: cases might not be wanted in there since we're basically poking at bits in the implementation in a non-C++ API-sanctioned way, but if it actually makes a difference then there's a case for it.
Nonetheless, handling it in the ptex or ptex-sys crates will at least prevent that as a possibility.
ratmice
These add (failing) tests for the funky values series, It appears that the ptex library doesn't check/truncate these values, as such they do end up being written to/read from the library.
I wasn't sure if that was perhaps something we should do in this crate in
ptextexture_get_border_mode_{u,v}andptextexture_get_edge_filter_mode, and if so what the defaults should be: I would supposeEdgeFilterMode::None, andBorderMode::Black?Or if this is something which should instead be done in the upstream library.