[Gimp-developer] Bug 155733 - discussion of approach (bug: need to check return values of gimp_drawable_mask_bounds())

[Neil <neildevbox gmail com>]

Hi...
Am just getting started with gimp hacking, and decided to pick a gnome-love bug to have a look at :)

I have a fairly clear idea of how to fix plugins which don't have any dialog box - they should just basically do nothing at all, and exit silently. (I'd have thought they ought to take the first opportunity to exit; I'm not sure this is true of the already-fixed ones but that's not the current issue.)

For plugins with dialog boxes, things are less obvious. I chose "grid" to look at first. I could change it in the fashion suggested in the bugzilla discussion, so that instead of calling gimp_drawable_mask_bounds() it calls gimp_drawable_mask_intersect() and checks the return value. However, this would still run the dialog, with a (totally spurious) preview. To me, it would make far more sense to bail out before the dialog is even called. However, when I had a look at blur_gauss.c, which is already fixed (i.e. it calls gimp_drawable_mask_intersect()), I saw that it does allow the plugin dialog to run.
In contrast, iwarp bails out immediately, with a warning that the affected region is empty (in a status bar below the image window).

I was tempted to infer that blur_gauss was fixed in a really minimalist way (just to avoid nasty stuff happening, e.g. crashes) while leaving the code in a slightly icky state (i.e. the dialog box running even though it can't do anything at all to the image), while iwarp was fixed in a "better" way. However, when I look at the git commit attributed to Luidnel Maignan (9b6c9e1fe4f46d2d47c6d97d4147cf060abd07f8) I can see that *both* approaches were used (iwarp was fixed in this patch, and a bunch of others too). So I'm confused!

Some guidance on which approach is best (and why) would be welcome...

ta
Neil