From bfd1274ed3ebbd7982683a013fa4177d5225f6e3 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 16 May 2017 10:10:11 -0700 Subject: [PATCH 1/3] Fix progressive loading of cubemaps In various places we were using the KTX _imageSize as the size of the entire mip level, when it is actually the size of the face for cubemaps. --- libraries/gpu/src/gpu/Texture_ktx.cpp | 7 ++++--- libraries/ktx/src/ktx/KTX.h | 19 +++++++++++-------- libraries/ktx/src/ktx/Writer.cpp | 9 ++++++--- .../src/model-networking/TextureCache.cpp | 8 ++++---- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 524fd0a88c..85fd5f044c 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -238,8 +238,9 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor throw std::runtime_error("Invalid level"); } - if (storage->size() != _ktxDescriptor->images[level]._imageSize) { - qWarning() << "Invalid image size: " << storage->size() << ", expected: " << _ktxDescriptor->images[level]._imageSize + auto& imageDesc = _ktxDescriptor->images[level]; + if (storage->size() != imageDesc._fullImageSize) { + qWarning() << "Invalid image size: " << storage->size() << ", expected: " << imageDesc._fullImageSize << ", level: " << level << ", filename: " << QString::fromStdString(_filename); return; } @@ -258,7 +259,7 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor return; } - memcpy(imageData, storage->data(), _ktxDescriptor->images[level]._imageSize); + memcpy(imageData, storage->data(), storage->size()); _minMipLevelAvailable = level; if (_offsetToMinMipKV > 0) { auto minMipKeyData = file->mutableData() + ktx::KTX_HEADER_SIZE + _offsetToMinMipKV; diff --git a/libraries/ktx/src/ktx/KTX.h b/libraries/ktx/src/ktx/KTX.h index 656cf93f34..69a34d3507 100644 --- a/libraries/ktx/src/ktx/KTX.h +++ b/libraries/ktx/src/ktx/KTX.h @@ -38,15 +38,15 @@ UInt32 numberOfArrayElements UInt32 numberOfFaces UInt32 numberOfMipmapLevels UInt32 bytesOfKeyValueData - + for each keyValuePair that fits in bytesOfKeyValueData UInt32 keyAndValueByteSize Byte keyAndValue[keyAndValueByteSize] Byte valuePadding[3 - ((keyAndValueByteSize + 3) % 4)] end - + for each mipmap_level in numberOfMipmapLevels* - UInt32 imageSize; + UInt32 imageSize; for each array_element in numberOfArrayElements* for each face in numberOfFaces for each z_slice in pixelDepth* @@ -269,7 +269,7 @@ namespace ktx { COMPRESSED_RG11_EAC = 0x9272, COMPRESSED_SIGNED_RG11_EAC = 0x9273, }; - + enum class GLBaseInternalFormat : uint32_t { // GL 4.4 Table 8.11 DEPTH_COMPONENT = 0x1902, @@ -419,17 +419,20 @@ namespace ktx { using FaceOffsets = std::vector; using FaceBytes = std::vector; + const uint32_t _numFaces; // This is the byte offset from the _start_ of the image region. For example, level 0 // will have a byte offset of 0. - const uint32_t _numFaces; const size_t _imageOffset; const uint32_t _imageSize; + // The full size of this image / mip level. This will be equivalent to _imageSize except when _numFaces > 1 + const uint32_t _fullImageSize; const uint32_t _faceSize; const uint32_t _padding; ImageHeader(bool cube, size_t imageOffset, uint32_t imageSize, uint32_t padding) : _numFaces(cube ? NUM_CUBEMAPFACES : 1), _imageOffset(imageOffset), - _imageSize(imageSize * _numFaces), + _imageSize(imageSize), + _fullImageSize(imageSize * _numFaces), _faceSize(imageSize), _padding(padding) { } @@ -466,7 +469,7 @@ namespace ktx { class KTX; - // A KTX descriptor is a lightweight container for all the information about a serialized KTX file, but without the + // A KTX descriptor is a lightweight container for all the information about a serialized KTX file, but without the // actual image / face data available. struct KTXDescriptor { KTXDescriptor(const Header& header, const KeyValues& keyValues, const ImageDescriptors& imageDescriptors) : header(header), keyValues(keyValues), images(imageDescriptors) {} @@ -495,7 +498,7 @@ namespace ktx { // Instead of creating a full Copy of the src data in a KTX object, the write serialization can be performed with the // following two functions // size_t sizeNeeded = KTX::evalStorageSize(header, images); - // + // // //allocate a buffer of size "sizeNeeded" or map a file with enough capacity // Byte* destBytes = new Byte[sizeNeeded]; // diff --git a/libraries/ktx/src/ktx/Writer.cpp b/libraries/ktx/src/ktx/Writer.cpp index 50f63767a0..3afe705499 100644 --- a/libraries/ktx/src/ktx/Writer.cpp +++ b/libraries/ktx/src/ktx/Writer.cpp @@ -89,7 +89,7 @@ namespace ktx { for (uint32_t l = 0; l < numMips; l++) { if (imageDescriptors.size() > l) { storageSize += sizeof(uint32_t); - storageSize += imageDescriptors[l]._imageSize; + storageSize += imageDescriptors[l]._fullImageSize; storageSize += Header::evalPadding(imageDescriptors[l]._imageSize); } } @@ -127,6 +127,7 @@ namespace ktx { size_t KTX::writeWithoutImages(Byte* destBytes, size_t destByteSize, const Header& header, const ImageDescriptors& descriptors, const KeyValues& keyValues) { // Check again that we have enough destination capacity if (!destBytes || (destByteSize < evalStorageSize(header, descriptors, keyValues))) { + qWarning() << "Destination capacity is insufficient to write KTX without images"; return 0; } @@ -149,13 +150,15 @@ namespace ktx { for (size_t i = 0; i < descriptors.size(); ++i) { auto ptr = reinterpret_cast(currentDestPtr); *ptr = descriptors[i]._imageSize; - ptr++; + #ifdef DEBUG + ptr++; for (size_t k = 0; k < descriptors[i]._imageSize/4; k++) { *(ptr + k) = 0xFFFFFFFF; } #endif - currentDestPtr += descriptors[i]._imageSize + sizeof(uint32_t); + currentDestPtr += sizeof(uint32_t); + currentDestPtr += descriptors[i]._fullImageSize; } return destByteSize; diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 47fc62a9b5..79b179f3b9 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -648,13 +648,13 @@ void NetworkTexture::maybeHandleFinishedInitialLoad() { // TODO Move image offset calculation to ktx ImageDescriptor for (int level = static_cast(images.size()) - 1; level >= 0; --level) { auto& image = images[level]; - if (image._imageSize > imageSizeRemaining) { + if (image._fullImageSize > imageSizeRemaining) { break; } - ktxData -= image._imageSize; - texture->assignStoredMip(static_cast(level), image._imageSize, ktxData); + ktxData -= image._fullImageSize; + texture->assignStoredMip(static_cast(level), image._fullImageSize, ktxData); ktxData -= ktx::IMAGE_SIZE_WIDTH; - imageSizeRemaining -= (image._imageSize + ktx::IMAGE_SIZE_WIDTH); + imageSizeRemaining -= (image._fullImageSize + ktx::IMAGE_SIZE_WIDTH); } // We replace the texture with the one stored in the cache. This deals with the possible race condition of two different From 7225326bfc0bce18993eac5f8f37ae4f98cc8855 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 16 May 2017 10:11:07 -0700 Subject: [PATCH 2/3] Fix failed mip insertions not being handled by TextureCache --- .../src/model-networking/TextureCache.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 79b179f3b9..2880762987 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -501,12 +501,19 @@ void NetworkTexture::ktxMipRequestFinished() { if (texture) { texture->assignStoredMip(_ktxMipLevelRangeInFlight.first, _ktxMipRequest->getData().size(), reinterpret_cast(_ktxMipRequest->getData().data())); - _lowestKnownPopulatedMip = _textureSource->getGPUTexture()->minAvailableMipLevel(); + + if (texture->minAvailableMipLevel() <= _ktxMipLevelRangeInFlight.first) { + _lowestKnownPopulatedMip = texture->minAvailableMipLevel(); + _ktxResourceState = WAITING_FOR_MIP_REQUEST; + } else { + qWarning(networking) << "Failed to load mip: " << _url << ":" << _ktxMipLevelRangeInFlight.first; + _ktxResourceState = FAILED_TO_LOAD; + } } else { + _ktxResourceState = WAITING_FOR_MIP_REQUEST; qWarning(networking) << "Trying to update mips but texture is null"; } finishedLoading(true); - _ktxResourceState = WAITING_FOR_MIP_REQUEST; } else { finishedLoading(false); if (handleFailedRequest(_ktxMipRequest->getResult())) { From e0212ac32bc824de8a2345f054b6e1b7a500695f Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 16 May 2017 15:53:07 -0700 Subject: [PATCH 3/3] Remove _fullImageSize from ktx header --- libraries/gpu/src/gpu/Texture_ktx.cpp | 4 ++-- libraries/ktx/src/ktx/KTX.h | 5 +---- libraries/ktx/src/ktx/Writer.cpp | 4 ++-- .../src/model-networking/TextureCache.cpp | 8 ++++---- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 85fd5f044c..e033355514 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -239,8 +239,8 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor } auto& imageDesc = _ktxDescriptor->images[level]; - if (storage->size() != imageDesc._fullImageSize) { - qWarning() << "Invalid image size: " << storage->size() << ", expected: " << imageDesc._fullImageSize + if (storage->size() != imageDesc._imageSize) { + qWarning() << "Invalid image size: " << storage->size() << ", expected: " << imageDesc._imageSize << ", level: " << level << ", filename: " << QString::fromStdString(_filename); return; } diff --git a/libraries/ktx/src/ktx/KTX.h b/libraries/ktx/src/ktx/KTX.h index 69a34d3507..3f220abac3 100644 --- a/libraries/ktx/src/ktx/KTX.h +++ b/libraries/ktx/src/ktx/KTX.h @@ -424,15 +424,12 @@ namespace ktx { // will have a byte offset of 0. const size_t _imageOffset; const uint32_t _imageSize; - // The full size of this image / mip level. This will be equivalent to _imageSize except when _numFaces > 1 - const uint32_t _fullImageSize; const uint32_t _faceSize; const uint32_t _padding; ImageHeader(bool cube, size_t imageOffset, uint32_t imageSize, uint32_t padding) : _numFaces(cube ? NUM_CUBEMAPFACES : 1), _imageOffset(imageOffset), - _imageSize(imageSize), - _fullImageSize(imageSize * _numFaces), + _imageSize(imageSize * _numFaces), _faceSize(imageSize), _padding(padding) { } diff --git a/libraries/ktx/src/ktx/Writer.cpp b/libraries/ktx/src/ktx/Writer.cpp index 3afe705499..2dcd0ea583 100644 --- a/libraries/ktx/src/ktx/Writer.cpp +++ b/libraries/ktx/src/ktx/Writer.cpp @@ -89,7 +89,7 @@ namespace ktx { for (uint32_t l = 0; l < numMips; l++) { if (imageDescriptors.size() > l) { storageSize += sizeof(uint32_t); - storageSize += imageDescriptors[l]._fullImageSize; + storageSize += imageDescriptors[l]._imageSize; storageSize += Header::evalPadding(imageDescriptors[l]._imageSize); } } @@ -158,7 +158,7 @@ namespace ktx { } #endif currentDestPtr += sizeof(uint32_t); - currentDestPtr += descriptors[i]._fullImageSize; + currentDestPtr += descriptors[i]._imageSize; } return destByteSize; diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 2880762987..8683d56b6b 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -655,13 +655,13 @@ void NetworkTexture::maybeHandleFinishedInitialLoad() { // TODO Move image offset calculation to ktx ImageDescriptor for (int level = static_cast(images.size()) - 1; level >= 0; --level) { auto& image = images[level]; - if (image._fullImageSize > imageSizeRemaining) { + if (image._imageSize > imageSizeRemaining) { break; } - ktxData -= image._fullImageSize; - texture->assignStoredMip(static_cast(level), image._fullImageSize, ktxData); + ktxData -= image._imageSize; + texture->assignStoredMip(static_cast(level), image._imageSize, ktxData); ktxData -= ktx::IMAGE_SIZE_WIDTH; - imageSizeRemaining -= (image._fullImageSize + ktx::IMAGE_SIZE_WIDTH); + imageSizeRemaining -= (image._imageSize + ktx::IMAGE_SIZE_WIDTH); } // We replace the texture with the one stored in the cache. This deals with the possible race condition of two different