> I don't agree with this. Making the format description table mutable when the > only formats that are potentially unsupported due to patent issues are s3tc > variants makes no sense. S3TC formats *are* special. There is nothing to > generalize here.
Yes, I don't like this very much either. The immediate alternative is to have separate "is_supported" flags for externally-implemented formats, but this also doesn't look perfect to me. Another thing to look at is to remove both is_supported and the pack/unpack functions, and put them in a separate, possibly mutable, table. In some sense pack/unpack functionality does not really belong in the format description, since many interfaces are possible (for instance llvmpipe has another interface that is code-generated separately for SoA tiles). This last option, with a mutable format access table, seems conceptually the cleanest to me, but not sure. > Replacing the conditionals with a no-op stubs is a good optimization. > But attempting to load s3tc shared library from the stubs is unnecessary. > Stubs should have an assert(0) -- it is an error to attempt any S3TC > (de)compression when there's no support for it. The fundamental issue here seems to be: what to do if the application tries to read/write an unsupported format? Currently, unsupported formats have empty functions rather than assert(0), so I just kept with that convention. Since it is permissible to call other format functions without checking they are supported, I made S3TC work consistently with that, which requires on-demand loading upon format access. In general it seems to me that the fact that S3TC (or any other) formats are somehow special should be completely hidden to any user. This allows to write generic robust format-independent code. Explicit initialization or ad-hoc format checking goes counter to this, and requires to sprinkle code everywhere (for instance, I suspect the rbug texture-examination tools don't work right now in master on S3TC because they don't call util_format_s3tc_init). It might makes sense to make all unsupported formats assert(0). A C++ exception would be the perfect thing since you could catch it, but unfortunately we aren't using C++ right now. Another option that seems better to me is to have an util_format_get_functions that would either give you a pointer to a table of functions, or return NULL if unsupported, and make this the only way of accessing format conversions. This way, applications will naturally have to check for support before usage, and both GCC and a static checker can be told to flag an error if the util_format_get_functions return value is not checked for null before use. BTW, note that the indirect function calls are also generally slow, and we may want to switch Gallium to C++ and use C++ templates to specialize (and fully inline) whole algorithms for specific formats. llvmpipe and the code generation facilities lessen the need for this, but it might perhaps be worthwhile in some cases. This a wholly separate issue, but may be worth keeping in mind. ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
