Summary: | halftones and refinement handling | ||
---|---|---|---|
Product: | jbig2dec | Reporter: | gorac <drugo.pedrouvene> |
Component: | Rendering | Assignee: | Henry Stiles <henry.stiles> |
Status: | RESOLVED WONTFIX | ||
Severity: | normal | CC: | christinedelight.top85, htl10, masaki.ushizaka |
Priority: | P4 | ||
Version: | unspecified | ||
Hardware: | PC | ||
OS: | Windows XP | ||
Customer: | Word Size: | --- | |
Attachments: | the 7 changed files + 2 new files as a patch |
Description
gorac
2009-12-06 08:40:22 UTC
Hmm, most of the changes in the 8-9 files are re-indentation and adding/removng line-breaks, except these small sections: jbig2_mmr.c: jbig2_decode_generic_mmr_ex() jbig2_refinement.c: implicit_value(), mkctx0(), jbig2_symbol_dict.c: Created attachment 5750 [details]
the 7 changed files + 2 new files as a patch
The 7 changed files and 2 new files as a patch, for review purposes.
Most of the changes are re-indentations and adding/removing line breaks,
unfortunately, which make it difficult to review what is actually being
changed. I think about 3-4 of the changed files consists entirely of
white-space changes.
yes, I have reformatted sources with SourceFormatX. jbig2_generic.c - added typical prediction handling code jbig2_mmr.c/h - added jbig2_decode_generic_mmr_ex() routine jbig2_refinement.c - added typical prediction handling code jbig2_segment.c - enabled halftones handling jbig2_symbol_dict.c - fixed 2 bugs in jbig2_decode_symbol_dict() routine jbig2_text.c - fixed a bug in jbig2_decode_text_region() routine jbig2_halftone.c/h - implemented jbig2_decode_halftone_region() routine ..maybe you are blind. Gorac Gorac, The "maybe you are blind" comment is, IMHO, iappropriate. That notwithstanding, please reformat your patch to original jbig2dec standards to minimize the indentation only changes. While it is unfortunate that jbig2dec appears to have not followed the GS coding style standards (4 character tab stops) it is much easier for us to review a patch that only shows the actual changes. If, later, you or we decide to change the indentation, we will do that across all files, with an appropriate comment in the log "Only indentation changes" OK Ray, sorry to disturb you. Please, ignore my implementation. Take the first file - 'jbig2_generic.c - added typical prediction handling code' - git reports 857 lines has changed; and there are two sections marked with your name in it - (that's bad style, BTW, since Artifex owns the copyright, it is somewhat inappropriate to put your name down to a section of code to claim ownership - there are occasional names in gs source code, but mostly they are there for 'I am not sure about this, if this breaks in the future, ask me for why I do what I do here' reason, which is more acceptable) - and even those two sections consists mostly of indentation/white-space changes, with may be only about 20 lines of significant changes, at a glance. And that's me reading it carefully trying to see what is changed in 857 lines. The 'fixed 2 bugs' 'fixed a bug' entries probably also should be submitted separately with details. There are also stylistic changes, like replacing "if(!row)" by "if (row == 0)" - which should be stripped out. My comment 1 was incomplete because I didn't want to spend any more time going through a lot of white-space changes and other stylistic changes like this to see what is significant. I am the author of typical prediction handling code you see in jbig2_generic.c; copyright owner is now Artifex because I made a present. http://bugs.ghostscript.com/show_bug.cgi?id=690913 If You don'know keep your mouth shut. There is my name around the sources only to mark my changes to original code but this is too hard to understand for you.You are annoing me with your silly style matters without a single recompiling of the code: really you are the more smart programmer in the world! Ralph can you decide what should be done with this? Regardless, Gorac should be payed for 690913 as we discussed. Gorac, please be civil. We appreciate your efforts to improve jbig2dec, but insulting those trying to review your code is not appropriate. Hin-tak is also entirely correct; we prefer that changes be supplied as minimal patches, each providing a specific change. Henrys (comment #7), bug 690913 was closed as a duplicate of customer bug 690791, which is marked as bountiable. Gorac is eligible to claim the bounty on that bug, as discussed. The proposed change is not in the part of ghostscript for which I am the default owner of, so I'll let the default owner decide. There is a coding guideline in ghostscript/doc/C-style.htm . |