Crash when saving big file into ico file type

[grofaty]

Pinta 1.2 on Windows XP and latest development on Ubuntu 11.10:
1. Open Pinta.
2. New Image 800x600.
3. Draw something with Paintbrush.
4. File | Save As.
5. Name: test From file type drop-down window select: *.ico and click on Save button. Crash error appears:
===========
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> GLib.GException: Image too large to be saved as ICO
   at Gdk.Pixbuf.Save(String filename, String type)
   at Pinta.Core.GdkPixbufFormat.DoSave(Pixbuf pb, String fileName, String fileType)
   at Pinta.Core.GdkPixbufFormat.Export(Document document, String fileName)
   at Pinta.Actions.SaveDocumentImplmentationAction.SaveFile(Document document, String file, FormatDescriptor format)
   at Pinta.Actions.SaveDocumentImplmentationAction.SaveFileAs(Document document)
   at Pinta.Actions.SaveDocumentImplmentationAction.Activated(Object sender, DocumentCancelEventArgs e)
   at Pinta.Core.FileActions.RaiseSaveDocument(Document document, Boolean saveAs)
   at Pinta.Core.Document.Save(Boolean saveAs)
   at Pinta.Actions.SaveDocumentAsAction.Activated(Object sender, EventArgs e)
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle._InvokeMethodFast(IRuntimeMethodInfo method, Object target, Object[] arguments, SignatureStruct& sig, MethodAttributes methodAttributes, RuntimeType typeOwner)
   at System.RuntimeMethodHandle.InvokeMethodFast(IRuntimeMethodInfo method, Object target, Object[] arguments, Signature sig, MethodAttributes methodAttributes, RuntimeType typeOwner)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture, Boolean skipVisibilityChecks)
   at System.Delegate.DynamicInvokeImpl(Object[] args)
   at GLib.Signal.ClosureInvokedCB(Object o, ClosureInvokedArgs args)
   at GLib.SignalClosure.Invoke(ClosureInvokedArgs args)
   at GLib.SignalClosure.MarshalCallback(IntPtr raw_closure, IntPtr return_val, UInt32 n_param_vals, IntPtr param_values, IntPtr invocation_hint, IntPtr marshal_data)
===========

It looks image is too big to be saved in ico format. But there should be no crash, just an info message that ico is not supported for big file sizes.

[Robert Nordan (rpvn)]

Confirmed. This line in the GdkPixBuf source code ( http://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/io-ico.c#n998 ) says that .ico files are not expected to be more than 256 x 256. But would it be more prudent to check before trying to save and warn the user, or catch the exception coming back up and warn the user?

[grofaty (grofaty)]

It depends on how likely is that 256 x 256 limitation will be changed.
1. If you warn user before changing exception it is danger if 256 x 256 limitation is changed.
2. If you catch the exception you are immune to the limitation change.

First option is probably faster. But it is probably no big deal to use any of this two options, because saving in ico file is something end-users probably rarely do and will take very little time for extra call (option 2).

On the other hand beside putting a warning to the user, it would be nice to have a "Resize automatically" button in warning - you know just like if pasted image does not gets into the current sizes canvas (but this is vice versa problem) . But I don't know how big is a hassle to do such a "resizing". You also have to know that width and height could be non-equal, so resizing maximum of width and height.

Maybe just put an warning that image should be 256 x 256 and user should use resize image to properly resize image.

[grofaty (grofaty)]

Ah, now I have checked the link you provided there is a code:
if (icon->width > 255 || icon->height > 255)
So image must not be bigger then 255 x 255 and not 256 x 256 as stated in #1.

[Robert Nordan (rpvn)]

Hmm, you're right. I just assumed it said 256 because I know from earlier that that is a typical Windows icon size. However, Wikipedia[1] and Microsoft[2] prove that I'm right. This is actually a bug in GDK, because they don't respect the fact that 256 px sizes are to be recorded as 0. (And of course, why Microsoft chose to do it that way is a mystery.) But until they fix that (off to file a bug report now), we'll have to just go along with the wrong limit.

Anyway, I've decided to do the warning after the exception comes up, since checking before would only add an unnecessary step to the other 99% of use cases. Also no automatic resizing because users will probably want to have a bit more control over hos their icon is resized.

Fixed in master: https://github.com/PintaProject/Pinta/commit/6525eb32808bc38ac8ee624496903c381586a1ce and
https://github.com/PintaProject/Pinta/commit/bb476947294ee1146198921826b79e82e216ed6b

[1]: https://en.wikipedia.org/wiki/ICO_%28file_format%29
[2]: http://msdn.microsoft.com/en-us/library/aa511280.aspx

[grofaty (grofaty)]

Tested in pinta-1.3-preview-20120425 on Windows XP. Now dialog is opened with warning message. Problem fixed.