Re: [gnome-print] Filtering code

[Chema Celorio <chema ximian com>]

On Tue, 2002-06-18 at 04:49, Scott Gifford wrote:
> > I started integrating your filter work into gnome-print and did some
> > modifications to it to GNOMEfy it. As such the code worked fine and
> > there isn't anything wrong about not beeing GNOMEfied from a code
> > perspective it works fine, but having a consistent way of writing code
> > makes GNOME a lot easier to understand. I got a bit carried away but
> > decided tonight to give you what I currently have so you can continue
> > working on it.
> 
> Hrm...I have mixed feelings on this.  Once everything's working, I'm
> hoping I can share this code with the folks at Mozilla and KDE
> (neither of whom had decent postscript image encoding when I checked
> about a year ago), so I'm reluctant to add a dependency on glib.  On
> the other hand, as part of gnome-print it makes sense to be consistent
> with everything else...I'll have to contemplate a bit...

Understanding GNOME is hard, and having a consistent coding method is
helpful. About dependencies, Mozilla already depends on glib, but what
is more important is that this projects are also going to ask for
contributions to be consistent with what they use, mainly for the same
reasons we do so (I know for sure about Mozilla, but assume KDE will
too).

I think the important part is getting the algorithms right, the
boilerplate code is not the problem, the real juice comes from the
algorithms and the postscript dictionary to uncompress images.


> > This are some of the GNOMEisms I changed.
> 
> [...]
> 
> All of those look fine (modulo the above comment about GNOMEfying...)
> It's going to take me awhile to be able to understand the GNOMEfied
> code, since I don't really know anything about GNOME/GTK's object
> system beyond the very basics.  Is there a good tutorial on this
> somewhere?

GObject:
http://developer.gnome.org/doc/API/2.0/gobject/index.html
& gtk+
http://www.gtk.org/tutorial/

To be honest, I learned this by looking at code and how other hackers
where using it.

> > I didn't finished polishing the rle filter, but added some comments in
> > the code. I didn't tested the filter code for hex & rle, it was in
> > "compiles" state ;-). I don't think that i screwed anything up but I
> > can't bet my life on it. You'll find the patch unfinished and with some
> > leftover code too which we need to clean before getting comiting it.
> 
> The test suite should be able to confirm whether anything broke.  cd
> into tests/filter, and run ./test.  The test suite stuff was included
> in my original patch and tarfile.

ok.

> When I manage to get this to compile, I'll run through the test and
> check it out, too.
> 
> > On the short term this is what I'd like to do:
> > 
> > - Only get 3 filters working: hex, rle, & flate. flate is the most
> > important one because the objects and classes might need tweaking to get
> > this filter to work. Forget about the other filters for now lets get the
> > structs and API nailed first.
> 
> I don't know enough about compression and zlib to do flate without
> some help, which is why I didn't do it originally.  Any pointers?  Do
> we already have a dependency on zlib?

My old gnome-print code had filter for this, and yes we used zlib. We
can depend on zlib, almost everyone depends on it.

> Also, is there a reason to use hex instead of a85?  a85 is
> significantly smaller.

Well, i think flate is harder and might require us to change the
structure of GnomePrintFilter, we might need to save a "reminder" of
unfiltered bytes that the compression didn't used and that we need to
prepend to the next time we filter, I might be wrong it might just work
ok, but that is what I'd like to find out.

> > - We need to make the filters work both ways, encode and decode to make
> > the test suite reliable. Plus we already had code for this in the 1.4
> > branch.
> 
> In the test suite in tests/filters, I use ghostscript to do the
> decoding.  What do you think of that idea?

Excellent, whenever we can use someone else's code to help us the
better. I just ghotscript too on ./run-tests.pl ;-).

> > > - Forget about the _set_pointer and _set_string functions for now,
> > hardcode those values. We will use GPANodes later for it. For now, only
> > make it so that one filter works and it is passed as an id by the rest
> > of gnome-print
> 
> Any particular reason you'd like to see this code go?  It's currently

No, we need that functionality i was thinking of forgetting it for now
and add it later.

> used by the test suite for testing the various combinations of
> filters, and was designed with the idea that it would be easy to use
> for customizing printing options from a printer properties dialog box
> in the long-term, and from a simple environment variable in the
> short-term...Something like:
> 
>    GNOME_PRINT_IMAGE_ENCODE="rle a85"
> 
> for good compression, or
> 
>    GNOME_PRINT_IMAGE_ENCODE="hexenc"
> 
> for the old behavior.  This could be especially useful as a workaround
> for problems with this encoding, or to enable extra compression where
> needed if you know your printer supports it.

Absolutely, we need this. I just wanted to hook it up to the rest of the
configuration system that gnome-print has libgnomeprint/gpa, and we
might need to pass extra parameters to the filters, specially the
filters that are more complex.

Applications and gnome-print insternal can use GnomePrintConfig (and
GPANodes) to configure the filter parameters just as they do everything
else, this will allow us to expose to applications the filter options,
the application might know something more about images for example and
what method to use, or .. applications might want to add PDF Export
functionality (File->Export..) and tweak parameters.

So my thought was "Lets get this out of the way for now, when the rest
is settled we can come back to this." For example i added a member to
the hex filter for default line length break, we could make it
configurable in the future (although i think we might not need it)

> > - In the testsuite code, I'd like to decode files and decode them rather
> > than having known stuff. This makes our testing code scale, we don't
> > need to add more files when we port more filters.
> 
> In the test code in tests/filters, it encodes files, has ghostscript
> decode them, and makes sure they are identical.  Are you talking about
> something different than that?

Yes, I was talking about just testing the encoding/decoding by itself.
But ghotscript also solves the problem. We can certainly use this.

> 
> [...]
> 
> > Why did you used size_t for needed bytes instead of gint? I changed it
> > to gint but have the feeling it should really be size_t.
> 
> It's passed to malloc, which takes a size_t.

Oh, ok. I used g_malloc.

> Some comments on your inline comments:
> 
> In gnome_print_filter_rle_finish:
> 
>       /* FIXME, we shold have an enum GnomePrintFilterRleRunType rather than using 1 & -1
> 
> OK, although that seems to me to add complexity without any real gains...

True. Leave it as 1 & -1. It is borderline helpfull.
> 
>        * also why is 257 hardcoded below? <chema>
>        */
> 
> 257 is a constant from Adobe's "Filters and Reusable Streams", Sec. 4
> Paragaph 1.  It could be a constant with a name like
> GNOME_PRINT_FILTER_RLE_LENGTH_SUBTRACTFROM_CONSTANT, but I'm not sure
> that's much more illuminating... :-)

Constants defined are better. Add a reference to the documentation to
explain where that constant came from. Somebody might need to look at
this code in the future and will help him a lot.

>       /* If run_type and run_lenght are not beeing modified, why do we pass pointers to int
>        * rather than ints? <chema>
>        */
> 
> Yeah, there doesn't seem to be any reason for this.  IIRC, previous
> versions did the setting of *run_length and *run_type back to 0, but
> the caller is now responsible for that.

Ok.

>       /* what if we pass only to finish buff a GnomePrintFilterRle pointer? <chema>*/
> 
> That would require us to update the structure before calling it, so
> would probably have about the same overhead, but other than that I

True. So it might be better to leave it as it is, your call.

> don't see any problem with it.  finishbuf() started out as a simple
> factoring out of repetitive code in gnome_print_rle_filter, which is
> why it's written that way.
> 
> I'll work on getting this to compile (my environment seems messed up),
> and will probably have more comments then.

Sure.

> 
> Thanks,
> 
> ----ScottG.

Thanks,
Chema