Re: incorrect mask handling in histogram calculation
Roel Schroeven wrote: Austin Donnelly wrote: 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? If the other tools pass a null pointer for the mask, the histogram is still calculated for the entire layer, so I think this shouldn't be a problem. I have commited this fix (and also your median calculation fix) to both the stable and devel branches of gimp. The original behaviour resulted in a totaly corrupt the histogram if you tried to use it with a mask. I'm not sure if the tools should be modified to stop computing the histogram through the current mask or not. Clearly the original intent of the code was to use the mask in the calculation. Thanks for digging these bugs up, Jay Cox [EMAIL PROTECTED]
Re: incorrect mask handling in histogram calculation
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 (Image/Image/Colors/Auto/Equalize). 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? Austin
Re: incorrect mask handling in histogram calculation
Austin Donnelly wrote: 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? If the other tools pass a null pointer for the mask, the histogram is still calculated for the entire layer, so I think this shouldn't be a problem.
incorrect mask handling in histogram calculation
I just downloaded (and installed and build) the 1.2.1. sources, and I was looking at the histogram code (because I wonder why the histogram always looks totally different than histograms from Photoshop or Paint Shop Pro) and I saw something (unrelated to what I was doing) that looks like a bug. 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; Regards, Roel Schroeven