https://bugs.freedesktop.org/show_bug.cgi?id=84566
--- Comment #37 from Jason Ekstrand <ja...@jlekstrand.net> --- (In reply to Iago Toral from comment #35) > Jason, you wrote some fast paths in _mesa_format_convert for the cases where > we can directly pack or unpack to the destination format. In these paths, if > we have to pack from or unpack to RGBA8888 UINT, you have asserts to make > sure that we are packing to or unpacking from an integer type. Likewise, if > we are packing from or unpacking to RGBA8888 UBYTE, you added asserts to > make sure that we are packing to or unpacking from a non-integer type. > > I am not sure why these were added, but I think we should remove them since > there are valid conversions that will hit these paths. For example, > glReadPixels where we want to store the pixel data in > GL_RGBA/GL_UNSIGNED_INT. In this case, the destination format matches one of > these fast path (RGBA UINT) but the source format, which is the render > buffer's format, is MESA_FORMAT_B8G8R8A8_UNORM which is not integer. The reason for this is that the original set of [un]packing functions only included functions for [un]packing integer types to/from uint and [un]packing normalized types to/from ubyte. Any normalized type can (in theory) be converted to a float. The ubyte versions existed to speed things up when the format was small enough that [un]packing to/from ubyte wouldn't lose any precision. In theory, we could also do [un]packing to/from 16 and 32-bit normalized types and that would be slightly faster than going through float. I didn't do this for a couple of reasons 1) Even though autogenerated functions are cheap in the sense that they require almost no effort to code, they do increase the size of the library. If every format had 4 packing and 4 unpacking functions, that makes for a fairly large amount of code. 2) Most packed unorm formats have fewer than 8 bits per pixel. The only exception I can think of off hand is 10-10-10-2. Therefore, most unorm conversions can either go through ubyte, will hit the swizzle_and_convert path, or will have to go through float anyway. I don't think we really gain much by having them go through uint 3) For integer types, GL only allows conversion to/from float and to/from other integer (not unorm) types. Since all integer types can fit in 32 bits, uint32 is a natural intermediate format (signed integers get sign-extended). 4) Even moreso than with unorm, the vast majority of integer conversions will go through mesa_swizzle_and_convert. I belive the only packed integer format is 10-10-10-2. We need to have the unpacking functions for the odd conversion case, but most won't hit it so also allowing packing to/from lower numbers of bits doesn't gain us much. > I think we have at least two options: > > 1) Transform the asserts into conditionals. With these we make sure that > these fast paths are only used when the assertions you have are not > violated, but we still let _mesa_format_convert use the non-fast paths to > resolve the conversion in the cases that would hit the assertions. If there > was a good reason for these assertions to exist then I guess this is the way > to go. I think this is the one we want, but I'm not 100% sure I understand what you mean. > 2) We simply remove the asserts. I think pack/unpack functions can deal with > any format type, right? At the moment uint pack/unpack functions seem to be > an exception since they only support packing to and unpacking from integer > types, but since these functions are auto-generated it seems we could just > let them handle other types too. Since in this case we would enable the fast > path, it would be the most efficient solution. > > What do you think? Given what I said above, does it make more sense now? It's quite possible that I've missed something, but that explains my reasoning. -- You are receiving this mail because: You are the QA Contact for the bug.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
