diff --git a/libs/image/kis_image_config.cpp b/libs/image/kis_image_config.cpp --- a/libs/image/kis_image_config.cpp +++ b/libs/image/kis_image_config.cpp @@ -43,6 +43,13 @@ : m_config( KSharedConfig::openConfig()->group(QString())), m_readOnly(readOnly) { +#ifdef Q_OS_OSX + // clear /var/folders/ swap path set by old broken Krita swap implementation in order to use new default swap dir. + QString swap = m_config.readEntry("swaplocation", ""); + if (swap.startsWith("/var/folders/")) { + m_config.deleteEntry("swaplocation"); + } +#endif } KisImageConfig::~KisImageConfig() @@ -221,7 +228,25 @@ QString KisImageConfig::swapDir(bool requestDefault) { +#ifdef Q_OS_OSX + // On OSX, QDir::tempPath() gives us a folder we cannot reply upon (usually + // something like /var/folders/.../...) and that will have vanished when we + // try to create the tmp file in KisMemoryWindow::KisMemoryWindow using + // swapFileTemplate. thus, we just pick the home folder if swapDir does not + // tell us otherwise. + + // the other option here would be to use a "garbled name" temp file (i.e. no name + // KRITA_SWAP_FILE_XXXXXX) in an obsure /var/folders place, which is not + // nice to the user. having a clearly named swap file in the home folder is + // much nicer to Krita's users. + + // furthermore, this is just a default and swapDir can always be configured + // to another location. + + QString swap = QDir::homePath(); +#else QString swap = QDir::tempPath(); +#endif return !requestDefault ? m_config.readEntry("swaplocation", swap) : swap; } diff --git a/libs/image/tiles3/kis_tile_data_store.cc b/libs/image/tiles3/kis_tile_data_store.cc --- a/libs/image/tiles3/kis_tile_data_store.cc +++ b/libs/image/tiles3/kis_tile_data_store.cc @@ -259,8 +259,12 @@ if(td->data()) { unregisterTileDataImp(td); - m_swappedStore.swapOutTileData(td); - result = true; + if (m_swappedStore.trySwapOutTileData(td)) { + result = true; + } else { + result = false; + registerTileDataImp(td); + } } td->m_swapLock.unlock(); diff --git a/libs/image/tiles3/swap/kis_memory_window.h b/libs/image/tiles3/swap/kis_memory_window.h --- a/libs/image/tiles3/swap/kis_memory_window.h +++ b/libs/image/tiles3/swap/kis_memory_window.h @@ -66,13 +66,14 @@ private: - void adjustWindow(const KisChunkData &requestedChunk, + bool adjustWindow(const KisChunkData &requestedChunk, MappingWindow *adjustingWindow, MappingWindow *otherWindow); private: QTemporaryFile m_file; + bool m_valid; MappingWindow m_readWindowEx; MappingWindow m_writeWindowEx; }; diff --git a/libs/image/tiles3/swap/kis_memory_window.cpp b/libs/image/tiles3/swap/kis_memory_window.cpp --- a/libs/image/tiles3/swap/kis_memory_window.cpp +++ b/libs/image/tiles3/swap/kis_memory_window.cpp @@ -27,17 +27,30 @@ : m_readWindowEx(writeWindowSize / 4), m_writeWindowEx(writeWindowSize) { - QString swapFileTemplate = (swapDir.isEmpty() ? QDir::tempPath() : swapDir) + QDir::separator() + SWP_PREFIX; - QDir d(swapDir.isEmpty() ? QDir::tempPath() : swapDir); + m_valid = true; + + // swapDir will never be empty, as KisImageConfig::swapDir() always provides + // us with a (platform specific) default directory, even if none is explicity + // configured by the user; also we do not want any logic that determines the + // default swap dir here. + Q_ASSERT(!swapDir.isEmpty()); + + QDir d(swapDir); if (!d.exists()) { - d.mkpath(swapDir.isEmpty() ? QDir::tempPath() : swapDir); + m_valid = d.mkpath(swapDir); } - m_file.setFileTemplate(swapFileTemplate); - bool res = m_file.open(); - Q_ASSERT(res); - Q_ASSERT(!m_file.fileName().isEmpty()); - if (!res || m_file.fileName().isEmpty()) { - qWarning() << "Could not create or open swapfile"; + + if (m_valid) { + QString swapFileTemplate = swapDir + QDir::separator() + SWP_PREFIX; + qWarning() << "trying to create swap file at " << swapFileTemplate; + m_file.setFileTemplate(swapFileTemplate); + bool res = m_file.open(); + if (!res || m_file.fileName().isEmpty()) { + m_valid = false; + } + } + if (!m_valid) { + qWarning() << "Could not create or open swapfile; disabling swapfile"; } } @@ -47,19 +60,23 @@ quint8* KisMemoryWindow::getReadChunkPtr(const KisChunkData &readChunk) { - adjustWindow(readChunk, &m_readWindowEx, &m_writeWindowEx); + if (!adjustWindow(readChunk, &m_readWindowEx, &m_writeWindowEx)) { + return nullptr; + } return m_readWindowEx.calculatePointer(readChunk); } quint8* KisMemoryWindow::getWriteChunkPtr(const KisChunkData &writeChunk) { - adjustWindow(writeChunk, &m_writeWindowEx, &m_readWindowEx); + if (!adjustWindow(writeChunk, &m_writeWindowEx, &m_readWindowEx)) { + return nullptr; + } return m_writeWindowEx.calculatePointer(writeChunk); } -void KisMemoryWindow::adjustWindow(const KisChunkData &requestedChunk, +bool KisMemoryWindow::adjustWindow(const KisChunkData &requestedChunk, MappingWindow *adjustingWindow, MappingWindow *otherWindow) { @@ -103,7 +120,9 @@ Q_UNUSED(otherWindow); #endif - m_file.resize(newSize); + if (!m_file.resize(newSize)) { + return false; + } #ifdef Q_OS_WIN32 if (otherWindow->chunk.size()) { @@ -120,5 +139,11 @@ adjustingWindow->window = m_file.map(adjustingWindow->chunk.m_begin, adjustingWindow->chunk.size()); + + if (!adjustingWindow->window) { + return false; + } } + + return true; } diff --git a/libs/image/tiles3/swap/kis_swapped_data_store.h b/libs/image/tiles3/swap/kis_swapped_data_store.h --- a/libs/image/tiles3/swap/kis_swapped_data_store.h +++ b/libs/image/tiles3/swap/kis_swapped_data_store.h @@ -48,7 +48,7 @@ * LOCKING: the lock on the tile data should be taken * by the caller before making a call. */ - void swapOutTileData(KisTileData *td); + bool trySwapOutTileData(KisTileData *td); /** * Restore the data of a \a td basing on information diff --git a/libs/image/tiles3/swap/kis_swapped_data_store.cpp b/libs/image/tiles3/swap/kis_swapped_data_store.cpp --- a/libs/image/tiles3/swap/kis_swapped_data_store.cpp +++ b/libs/image/tiles3/swap/kis_swapped_data_store.cpp @@ -55,7 +55,7 @@ return m_allocator->numChunks(); } -void KisSwappedDataStore::swapOutTileData(KisTileData *td) +bool KisSwappedDataStore::trySwapOutTileData(KisTileData *td) { Q_ASSERT(td->data()); QMutexLocker locker(&m_lock); @@ -75,12 +75,18 @@ KisChunk chunk = m_allocator->getChunk(bytesWritten); quint8 *ptr = m_swapSpace->getWriteChunkPtr(chunk); + if (!ptr) { + qWarning() << "swap out of tile failed"; + return false; + } memcpy(ptr, m_buffer.data(), bytesWritten); td->releaseMemory(); td->setSwapChunk(chunk); m_memoryMetric += td->pixelSize(); + + return true; } void KisSwappedDataStore::swapInTileData(KisTileData *td) @@ -96,6 +102,7 @@ td->setSwapChunk(KisChunk()); quint8 *ptr = m_swapSpace->getReadChunkPtr(chunk); + Q_ASSERT(ptr); m_compressor->decompressTileData(ptr, chunk.size(), td); m_allocator->freeChunk(chunk);