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



On 01/03/12 01:09, Neil wrote:
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


_____

Correction to my last comment, I just checked and sharpen does seem to work but blur has no dialog , it just does a blur with an uncontrolled radius.

The ensuing "Reshow Blur" had nothing to reshow and just adds another uncontrolled blur.

:?





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]