[sldev] Bug in LLTextureCacheRemoteWorker::doRead ?

[Carlo Wood]

I'm trying to understand the texture pipeline by
reading the code (unfortunately there is no documentation).

Doing so, I ran into the following that appears to
be a bug.

The class LLTextureCacheRemoteWorker has an
element called mOffset.

Because of code like this:

    mState = mOffset < TEXTURE_CACHE_ENTRY_SIZE ? HEADER : BODY;

it seems that mOffset is the data that was read so far.

Note that there are two sizes: data size (the whole image)
and file size. Those two differ because the first
TEXTURE_CACHE_ENTRY_SIZE bytes (600) are written to the
texture.cache file and the remaining bytes are written
to the cache/textures/?/* files.

So again, I think that mOffset is the *data* that was read
previously.

A bit further in LLTextureCacheRemoteWorker::doRead in
we see the following code however:

        if (mState == BODY)
	{
                std::string filename = mCache->getTextureFileName(mID);
                S32 filesize = ll_apr_file_size(filename, mCache->getFileAPRPool());
                S32 bytes_read = 0;
                if (filesize > mOffset)
                {
                        S32 datasize = TEXTURE_CACHE_ENTRY_SIZE + filesize;
                        mDataSize = llmin(datasize, mDataSize);
                        S32 data_offset = TEXTURE_CACHE_ENTRY_SIZE - mOffset;
                        data_offset = llmax(data_offset, 0);
                        S32 file_size = mDataSize - data_offset;
                        S32 file_offset = mOffset - TEXTURE_CACHE_ENTRY_SIZE;
                        file_offset = llmax(file_offset, 0);
...

Clearly, 'filesize' is the size of the "cache/textures/?/*" file
(the BODY), and thus 'datasize' is TEXTURE_CACHE_ENTRY_SIZE + filesize.

data_offset has mOffset subtracted, again indicating that mOffset is
a DATA offset, not a file offset.

The (remaining) file_size is then indeed mDataSize - data_offset,
and 'file_offset' is set to 'mOffset - TEXTURE_CACHE_ENTRY_SIZE',
again very clearly indicating that mOffset is the total (data) offset.

Therefore... the   '  if (filesize > mOffset)  '    must be a bug!

You can't compare the filesize (the size of the "cache/textures/?/*" file
with a data offset!

For example, at this point we could have read 900 bytes already
before so that mOffset == 900. The total data size then can be
anything >= 900, and thus filesize can be anything >= 300.
The result of that condition seems therefore rather arbitrary.

If the condition fails, mDataSize is set to 600 bytes and that's
it... So, an image of 900 bytes would get cut off at 600 bytes
if mOffset is > 300 at that point... That can't be right, right?

Anyone?

-- 
Carlo Wood <carlo at alinoe.com>