When float16 0x8400 is fed to BC6H compressor, glitches occur about ispctexturecompressor

[MarcFauconneau]

Yes, inputs aren't clamped. The compressor expects valid (positive FP16) values
Generally speaking, it seems preferable for the compressor to assume inputs are valid, so as not to occur any overhead to users with valid inputs (likely the overwhelming majority)

Is there a reason you can't clamp on your side before feeding data to the compressor?

[bmcnett]

Thanks Marc, for getting back to me so quickly.

It's simple enough for us to clamp on our side, thanks.

BTW - I observed this behavior in your own GUI tool - when I loaded a
float16 DDS file with 0x8400 values and compressed it, highly saturated
artifacts appeared.
Perhaps your GUI tool should clamp before feeding data to the compressor,
too.

Bryan

On Thu, Dec 3, 2015 at 10:31 AM, Marc Fauconneau [email protected]
wrote:

Yes, inputs aren't clamped. The compressor expects valid (positive FP16)
values
Generally speaking, it seems preferable for the compressor to assume
inputs are valid, so as not to occur any overhead to users with valid
inputs (likely the overwhelming majority)

Is there a reason you can't clamp on your side before feeding data to the
compressor?

—
Reply to this email directly or view it on GitHub
#2 (comment)
.

[MarcFauconneau]

Good point. Will do in next release. Thanks

[bmcnett]

I should make one comment before I move on: my preferred approach to compressing a DDS file is to, when possible, memory-map the file as read-only, and then feed the pointer to the compressor. If the compressor doesn't clamp inputs, I must instead make a copy of the DDS in memory, which in extreme cases can be 100 - 1000 MB, before I compress. Or, I guess, carefully maintain a system of smaller staging areas in memory. Or figure out copy-on-write semantics in the virtual memory API.

Arguably, it is simpler and more efficient to execute the half-dozen instructions to clamp inside the compressor, rather than to make the user pass potentially 100s of MB through the cache first, or et cetera.

[punkUser]

Curiously is there a reason you can't clamp the actual input file itself (or whatever generates it)? Is there some significance to the negative numbers that is relevant when fed into other tools that don't clamp inputs?

[bmcnett]

The issue is not that clamping is impossible or even difficult for a user to do - but rather that user-clamping requires passing the entire (potentially gigabytes) input through the cache hierarchy at least once more than is necessary, in the case where the input is already a FLOAT16 DDS. This may not be an issue for most users, but it is a concern of mine.

The half-dozen instructions necessary for clamping in the compressor is, in the grand scheme of things, cheap, when compared to passing the entire input through the memory hierarchy once or more.

[punkUser]

Sorry I guess my question wasn't clear - how are these values getting into the DDS file on disk in the first place, and is there any real reason you can't clamp them on disk (when they are being written the first time or as a separate step)?

The skepticism here isn't around whether or not it makes sense for your use case - obviously it does and it's easy enough to fork or parameterize the code to do that if you want. It's more that this code is being used in production pretty broadly already and no one else has reported running into this that I know of, so it's sort of undesirable to slow things down for everyone if they don't need this behavior.

Anyways this is all pretty minor given the source code is readily available - just trying to do the best thing for the broad set of folks already using it.

[bmcnett]

If my tool purports to load DDS files, it must accept anything that meets the DDS specification. Users of my tools may or may not clamp FLOAT16 values before writing to a DDS file, and this happens outside of my knowledge and control. Because I purport to support DDS, I must accept negative values as input, and because the compressor glitches on negative values, I must make a copy of the data (which may be 2^41 texels!) clamp it, and pass it to the compressor.

This is not the end of the world - it's a simple job - but it's unfortunate that I am forced to allocate and copy potentially gigabytes of data, so that a compressor can avoid executing so many dozen ALU instructions. This doesn't sound like a performance win to me.

[punkUser]

There's a DDS specification? :)

Kidding aside if you see these inputs with your asset pipeline then indeed you will have to handle them somehow. If it's not possible to handle them at generation time, I'd recommend you simply fork the repo and add the appropriately clamping. That's why the source is available for free :) For you, this makes a lot of sense, but I'm still not convinced that the broader audience would appreciate any additional performance cost for an issue they are not hitting.

Note that the expected behavior of a tool that compresses to BC6H in the presence of negative numbers would arguably be to compress to signed BC6H - which this compressor does not current support. I would imagine clamping is something you'd always want the user of the tool to be aware of as you are throwing away data that could have meaning.

Marc can comment if there's an easy way to efficiently parameterize the encoder one way or another but I'm sure he'd be happy to look at a pull request to that end as well.