Re: [Plplot-devel] plimagefr coordinate transform support - complete

[Hezekiah M. C. <hc...@at...>]

On Wed, Mar 19, 2008 at 2:31 PM, Andrew Ross
<and...@us...> wrote:
>
> On Wed, Mar 19, 2008 at 01:12:24PM -0500, Maurice LeBrun wrote:
>  > On Wednesday, March 19, 2008 at 10:14:30 (-0700) Alan W. Irwin writes:
>  >  > On 2008-03-19 10:59-0400 Hezekiah M. Carty wrote:
>  >  >
>  >  > > To sum up, I would like to submit patches in the follow steps:
>  >  > > (1) Add coordinate transform to plimagefr and disable the dev_fastimg
>  >  > > rendering path, but without removing the dev_fastimg code.
>  >  > > (2) Update dev_fastimg to work with the updated plimagefr, but only
>  >  > > use it for non-transformed images.
>  >  > > (3) Update example 20 with some examples of what plimagefr can do,
>  >  > > with pages to illustrate both fixed color ranges and coordinate
>  >  > > transformations.
>  >  > >
>  >  > > Does this sound like a reasonable compromise?
>  >  >
>  >  > Hi Hez:
>  >  >
>  >  > Actually after sleeping on it, I am leaning toward saying do (1) (with code
>  >  > commentary where you do the disabling in plimage.c, xwin.c, etc., about why
>  >  > it was necessary) and leave (2) as a would-be-nice rather than a requirement
>  >  > since it sounds like it might be a lot of work which you could more
>  >  > productively spend on the OCaml bindings, for example.
>  >  >
>  >  > However, I don't feel right making this decision alone because I haven't
>  >  > used -dev xwin or the plimage capability for my own PLplot needs, and
>  >  > somebody who has more of a vested interest in those parts of PLplot may feel
>  >  > a lot stronger about their speed than I do.  Thus, I am going to need
>  >  > advice/help from the other PLplot core developers on the decision about (1)
>  >  > and (2) so please step forward, guys, and comment.
>  >
>  > I use -dev xwin extensively (and its client, plframe) but not plimage.  That
>  > said, doing (1) and leaving/documenting (2) as a nice-to-have sounds fine to
>  > me, unless someone can make a case otherwise.
>
>  Ditto.
>
>
>  > Ideally someone would play with dev_fastimg vs !dev_fastimg and see if there's
>  > a noticeable difference on mondern hardware.  I'm sure many here have seen
>  > hardware advances make irrelevant some "optimizations" done previously, or at
>  > least mitigate performance concerns.
>
>  I agree. If it turns out that dev_fastimg really is useful then I would
>  encourage you to think about (2), i.e. still using dev_fastimg for the
>  no-transform case at least. Without the tests it is hard to comment
>  though.
>
>
>  > For example, the X driver was first developed on 8-bit r/w color displays and
>  > sharing a single r/w colormap was the norm.  If this didn't suffice for the
>  > application, a custom colormap could be used (which I never liked very much).
>  > Seems so quaint now. :) But when I went to 16 or 24 bit r/o colormaps on a
>  > Linux box years later, the performance degradation of swapping out colors
>  > really didn't seem to matter much.  One of these days I'd like to give the
>  > xwin driver a bit of housecleaning, starting with chopping out the custom
>  > colormap support that was never really used anyway.
>
>  If you can find the time, it sounds like a good idea! I still
>  extensively use the xwin driver. I prefer it over the more sophisticated
>  gnome driver for example for many purposes because it is quick and
>  clean.
>
>  Andrew
>
>  P.S. Hez, is the bug fix easy to tease out from the rest of your patch?
>  We should at least apply that as we think about the rest it. Committing
>  this separately also makes it clear what is bug fix and what is new
>  feature. This can be useful when looking back through the commits. If
>  the bug is only fixed because the broken code is replaced, then don't
>  worry about it.

Several updates have been made to the PLplot Subversion source tree
since I submitted my previous patches.  Here is an updated version of
my coordinate transform support in plimagefr patch which should apply
cleanly against the latest PLplot SVN revision (8382 as of this
writing).

To recap, the changes are:
- Comment out the dev_fastimg rendering path
- Add coordinate transform support to plimagefr (same structure as in
the plshade functions)
- Update api.xml with the above changes

Some code was hoisted out of plimagefr and back in to plimage to clean
things up a bit.  I also changed all of the comment to C style
comments /* */ rather than C++ style // to match the rest of the
PLplot code base.

I ran some tests using example 20 compiled against PLplot 5.9.0 vs
PLplot svn + this patch using the xwin display device to see what sort
of difference the removal of the dev_fastimg rendering path makes.
The difference in speed seems minimal if it exists at all.  This is on
a Pentium-M 1.7ghz laptop under Debian Sid.

Please let me know if there are any concerns with these changes.

Thanks,
Hez

-- 
Hezekiah M. Carty
Graduate Research Assistant
University of Maryland
Department of Atmospheric and Oceanic Science