After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 620604 - Description of "histogram" procedure is slightly inaccurate
Description of "histogram" procedure is slightly inaccurate
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: libgimp
git master
Other All
: Normal trivial
: 2.6
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2010-06-04 21:45 UTC by Krzysztof Kotlenga
Modified: 2010-06-05 17:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A patch (660 bytes, patch)
2010-06-04 21:46 UTC, Krzysztof Kotlenga
rejected Details | Review

Description Krzysztof Kotlenga 2010-06-04 21:45:47 UTC
Hello gimp devs.
The docs of "gimp-histogram" procedure say:
start-range  INT32  0 <= start-range <= 256
end-range    INT32  0 <= end-range <= 256

I think this is kinda weird since in the code values of these params are clamped to 255.
Comment 1 Krzysztof Kotlenga 2010-06-04 21:46:42 UTC
Created attachment 162773 [details] [review]
A patch
Comment 2 Michael Natterer 2010-06-05 09:21:56 UTC
While this fixes *this* bug, the actual bug is that the conversion
from the procedure's perl description to the generated code
in app/pdb/color-cmds.c is broken. It doesn't seem ho honor the
difference between < and <=. This should be fixed instead.
Comment 3 Michael Natterer 2010-06-05 09:22:32 UTC
Comment on attachment 162773 [details] [review]
A patch

Rejected because it fixed only a symptom but not the root of the problem
Comment 4 Krzysztof Kotlenga 2010-06-05 09:52:50 UTC
Yes, that's true, except that the patch would stay correct even if we fix pdbgen.pl and stuff. It's a low hanging fruit. I might have a look at pdbgen in a few weeks, but *this* bug could be fixed now.
Comment 5 Michael Natterer 2010-06-05 13:29:36 UTC
I agree, but let's keep it open as reminder anyway :)
Comment 6 Michael Natterer 2010-06-05 17:45:11 UTC
Fixed in master and 2.6:

commit 9f98dedd45fa4ddc1d58519b043fcf3ea4eae04a
Author: Michael Natterer <mitch@gimp.org>
Date:   Sat Jun 5 19:26:06 2010 +0200

    Bug 620604 - Description of "histogram" procedure is slightly inaccurate
    
    Fix totally broken value ranges of integer PDB parameters. Magically,
    the bug was affecting only exactly the two cases mentioned in above
    bug report.
    
    * tools/pdbgen/pdb.pl (arg_parse): return <, <=, > and >= literally
      instead of applying a mapping that was originally meant for
      generated C code that would e.g. transform "0 <= int32 < 10" into
      "if (value < 0 || value >= 10) fail". This inversion of all
      operators is now wrong because PDB parameters have been turned into
      GParamSpecs which always need inclusive ranges as min and max
      values.
    
    * tools/pdbgen/pdbgen.pl (arrayexpand): generated array length type
      specs must be "0 <= int32", not "0 < int32".
    
    * tools/pdbgen/app.pl: when generating integer param specs, check if
      the value range is specified in terms of < instead of <=, and
      add/subtract 1, resuting in the inclusive range needed for integer
      GParamSpecs.
    
    * app/pdb/color-cmds.c: regenerated, fixing the two broken ranges
      mentioned in the bug report.
    (cherry picked from commit 9dd373d86e71ac69301e4cbfaaeb4bee0eb5be6a)

 app/pdb/color-cmds.c   |    4 ++--
 tools/pdbgen/app.pl    |   42 ++++++++++++++++++++++++++++++++++++------
 tools/pdbgen/pdb.pl    |   16 +---------------
 tools/pdbgen/pdbgen.pl |    4 ++--
 4 files changed, 41 insertions(+), 25 deletions(-)