Saving a cropped image that is unsupported by Qt destroys the image

[philja]

Prompted by the other thread about 'AdjustImage feature improvements', I tried using the original crop facility on my MacBook using Sigil 2.8.1.

Note this does not concern DNSB's new Windows build.

After cropping a gif image, 'Save As' saves a version of the original image to disk (ignoring the crop operation).

'Save Changes' followed by 'Save As' saves a zero byte file to disk.

'Save Changes' also gives the unsupported file type error message for the gif but, more seriously, it destroys the original in the Images folder of the epub.

Sigil gives no indication of a change to the epub. Only a reload of the Preview window shows the image has gone.

Cmd-Z restores a visual display of the original uncropped image in a Code View tab. But reloading Preview' does not bring it back. Closing the image tab and opening a new tab does not display the image - it has been destroyed.

The least you'd expect would be that modifications to unsupported file types should just be ignored.

[DiapDealer]

Can confirm on Linux that the "save image changes to epub" action is somehow destroying the gif in Sigil's scratch area. I haven't determined if the crop attempt is required to trigger it or not. But there's no need to do a Save/Save As of the epub. Examining the scratch area after the "save image changes" is enough to reveal the zero byte file. Just closing the epub does not write the changes (or prompt that changes have been made). This is with Sigil 2.8.1.

It's a pretty neat trick for a QtImageIO plugin that claims to be read-only.

[DiapDealer]

Cropping is an unnecessary step. The gif image on disk is destroyed just as soon as the "Save Image Changes" button is clicked. But a Save/Save as (no unsaved changes prompt) is required to write the change back to the epub.

[DiapDealer]

Kevin's latest change (using Python to save the gif) definitely solves that problem for me (on Linux), but When saving the gif back to the epub (using the AdjustImage Save button), the epub is not flagged as modified and doesn't prompt to be saved when closing Sigil. Isn't it supposed to be? Saving other image formats (from within the AdjustImage widget) sets Sigil's modified flag.

[philja]

I've no problem in principle with a file format not being supported. But in that case, it would be better if the application could say so before allowing any modification of the file having that format.

The way it is at the moment means that if the image edit is made among a bunch of other edits to text files, and you save the epub to capture your other edits, the image file you've not been able to modify gets stuffed without warning.

Inability to edit gif's was never a problem for me. I just got into it as a result of testing the other thread about improvements to the image edit ability.

But, this has made me wonder what other file formats may not be supported and could suffer a similar fate.

[KevinH]

I have saved many cropped images using AdjustImage previously.

Are we only talking about gif images here or all images?

Or did the new rubber band code for cropping break something?

Is this with or without the gif writer built extension to use PIL?

Some more info here would certainly help.

[KevinH]

So is this fixed by the Python routine?

I will tweak it to make even gif writes in python tell the book is modified.

[KevinH]

And yes any time you try to write to a file and the the middle of the file write operation fails, the in disk file has typically been reset to hold nothing (it is called truncation on write). That is how openwith works and almost all file saves work.

Now you can try to use "atomic writes" where you create a temp file in the same directory as the original file and write to the temp file and only if successful, delete the original file and just rename the temp to it.

I could try to implement atomic writes in the image editor, but the act of deleting a file can fail and can signal Sigil that that the file is gone triggering removal of it in the manifest.

But with modern file systems, local writes just don't fail (unless your disk is corrupt or full and then you have bigger issues).

Again using Checkpoints is always the best idea before making destructive changes.

I updated the thread title to reflect this only happens with Qt unsupported image formats.

[DiapDealer]

The problem was only ever with gifs. The new rubber band cropping did not break anything. This was present in Sigil 2.8.1 before the new rubber band subclass was introduced.

Your latest python routine changes fixes everything (with exception of the modified flag that you have mentioned you are going to fix).

[KevinH]

Whew! Thanks for letting me know.

[philja]

Yes Kevin, it was only the gif's that had the problem.

I only tried it out on the MacBook because editing images inside Sigil was something I had never done before, and it was Sunday morning and the laptop was alongside the bed so it seemed a good idea to give it a go after having discovered this issue yesterday in the other thread.

The jpg's saved ok and the edited image showed the changes in Preview after updating the window. But no asterisk appeared on the title bar. Sigil gave no indication of any change so closing the epub containing the edited jpg did not get an invite to save the file. As expected, without saving the epub file, the changes to the image were lost.

I'm well aware of Checkpoints, thanks. And this image edit issue is not something that is going to affect me every day.

But I do wonder about the principle of warning the user as soon as he opens a file of an unsupported type in a tab that can be used for editing. It would be safer, if that is possible, to warn him at the point of opening the file that it is not supported for editing.

[KevinH]

That gif format was supported in Read only, which was news to me until your bug report and then I discovered it in the Qt source code.

GIFs are simply a strange Qt quirk and gifs for publishing have almost completely been replaced by PNG as a superior file format (which is why no one detected the bug before).

I will fix the marking of book modified.

[KevinH]

Okay, I have added in code to parallelize the various paths through AdjustImage doSave so that the signal to set book modified always fires if the save is successful.

I also aborted the save for and "PBM" or "PGM" images since they are all Read Only in Qt but are not part of the base mediatypes supported in epub. No write is attempted so no image loss can happen for these more obscure types.

I also added a message to re add the existing image for safety if image Save fails - which should really never happen anymore unless the user's disk is messed up, they are out of space, etc.

If need be, I can try to add an atomic save feature if anyone runs into any trouble with this new version but it would fail if the user's disk is full or messed up as well.

This has all been pushed to master.

Please give it a try and let me know if you see any other irregularities.

[DNSB]

Anyone running Windows and interested in testing this compile can download Sigil-2.8.5-Windows-x64-Setup.exe.

The SHA-256 for this compile of Sigil-2.8.5-Windows-x64-Setup.exe is:
5B55EA1D442BE30CBDB5C83B9462A63DB52B29C76C7A518729 EEA37B9E56C0B5

I also updated the link in the last message in the AdjustImage Feature Improvements thread.