Fix rename optimization during save #1111
Conversation
During saving, there was some code to check if a frame changed position but was not modified so that a simple file rename could be used instead of a full image render and write. However, there were many bugs that prevented this from working properly. I hope I have fixed them all. The most major one was caused by changes to the image loading, which would cause all frames to be loaded into memory during save. This fix will reduce memory consumption and increase the save speed by fixing this.
|
@scribblemaniac Hopefully the others can review the code as well as I can only help by testing. |
|
@scribblemaniac Hmm no I still can't build it after correcting my own mistake. Sorry |
|
@Jose-Moreno |
| QDir sA, sB; | ||
| if ((sA=QFileInfo(b->fileName()).dir()) != (sB=dataFolder)) { | ||
| // Copy instead of move if the data folder itself has changed | ||
| QFile::copy(b->fileName(), tmpName); |
There was a problem hiding this comment.
Could you tell me in what scenario the data folder will change?
There was a problem hiding this comment.
When you load a pclx file, and save it as a pcl file, the data folder will change from the temporary directory to the .data directory. @Jose-Moreno first noted problems arising from this scenario which appear to be fixed now.
|
Merged. Thanks @scribblemaniac |
During saving, there was some code to check if a frame changed position but was not modified so that a simple file rename could be used instead of a full image render and write.
However, there were many bugs that prevented this from working properly. I hope I have fixed them all. The most major one was caused by changes to the image loading, which would cause all frames to be loaded into memory during save. This fix will reduce memory consumption and increase the save speed by fixing this.
Thanks @Jose-Moreno for his extensive memory testing which brought this issue to my attention.
I have tested these changes in multiple scenarios and it is behaving as it should, but if I did make a mistake this could reintroduce the file wiping issues in the worst case scenario. So please review and test these changes thoroughly before merging.