On Saturday, 3 Feb 2001, Roel Schroeven wrote:

> In gimphistogram.c there is a function to calculate the histogram for a 
> subregion, declared as follows:
> gimp_histogram_calculate_sub_region (GimpHistogram *histogram,
>                     PixelRegion   *region,
>                     PixelRegion   *mask)
> In that function we have this code snippet:
>   if (mask)
>    {
>      gdouble masked;
>      src = region->data;
>      msrc = region->data;
> I would think that msrc ought to be a pointer into the mask data instead 
> of the region data, like this:
>    msrc = mask->data;

Yes, this is indeed a bug.

However, its not quite that simple.

The histogram code is used by a number of other tools: as part of the
histogram tool (<Image>/Image/Histogram...) but also as part of the
Levels tool (<Image>/Image/Colors/Levels...), the Threshold tool
(<Image>/Image/Colors/Threshold...) , and the Equalize tool

All except the histogram tool care about the current mask (ie
selection), since they restrict themselves to only operating on that
part of the image.

The histogram tool always gives the full histogram for the current
layer, ie it ignores the current selection.

So, fixing this bug means that the Levels tool, the Threshold tool,
and the Equalise tool will all also change their behaviour: currently
they use a histogram of the entire layer, but restrict their changes
to the current selection.  Fixing the bug means that the histogram
will only be calculated for within the selection too.  Is this the
desired behaviour?


Reply via email to