[GRASS5] d.legend updates

H_Bowman · November 29, 2002, 8:34am

I found myself using GIMP to place legends on raster images quite often, which didn't seem quite right, so I went ahead and wrote in the functionality that I wanted. And then I kept going..

New d.legend features include:

+ Legend text for floating point rasters is now much prettier.
      (%f -> %.2f, etc). Really tiny FP values convert to "2.4e-7" format.
+ at=x1,y1,x2,y2 implemented. Module is now scriptable / survives d.redraw
+ reimplemented -n flag: skip categories without labels. lines= takes on a new
      meaning when using this flag; means "up to category #". Its meaning becomes
      more blurry when you use lines=, thin=, and the -n flag together. This is
      a feature.
+ Floating point rasters now have the highest values at the top,
      lowest at the bottom.
+ Auto-scales text to fit in display window if bounds not manually set with
      at= or with mouse. Always auto-scales the "4 of 16 categories" text if
      needed.
+ You can now select the legend box with the mouse from any two corners
      and it will still work.
+ If you select a lower corner before a higher corner, the legend will
      reverse vertically. This also happens if the y values in at= are reversed.
      Works for horizontal legends as well.
+ fixed color-box off center within white frame for integer cats.
+ uses full legend box height to draw, even when using thin= or lines=.
+ better determines when to force a smoothed legend when using -n, lines=,
      &/or thin=.
+ smoothed legends with <5 categories now show <5 text labels.
+ Misc. code clean up & bug fixes, cleaned out some duplicate and
      unused code.

So, please try the new d.legend now in CVS, and let me know how badly you can break it, or what fine tuning you think it needs.

Hamish

p.s. - I've included a PNG of a legend I made with the PNG driver- it isn't quite right. Any ideas on if this would be a d.legend bug or a PNG driver bug? Both monitors were set to the same width and height (+/- 1 pixel). Does PNG size need to match res from g.region for non-lossy output??

(attachments)

Glynn_Clements · December 2, 2002, 5:10pm

H. Bowman wrote:

I found myself using GIMP to place legends on raster images quite often,
which didn't seem quite right, so I went ahead and wrote in the
functionality that I wanted. And then I kept going..

So, please try the new d.legend now in CVS, and let me know how badly you
can break it, or what fine tuning you think it needs.

Did you intentionally remove the -o and maxfontsize= options, or did
you base your updated version on old code (i.e. the 5.0.0 release
version instead of the CVS head)?

--
Glynn Clements <glynn.clements@virgin.net>

H_Bowman · December 3, 2002, 3:43am

On Mon, 2 Dec 2002 17:10:23 +0000
"Glynn Clements" <glynn.clements@virgin.net> wrote:

> the new d.legend now in CVS

Did you intentionally remove the -o and maxfontsize= options, or did
you base your updated version on old code (i.e. the 5.0.0 release
version instead of the CVS head)?

I based my version on what was in the Grass 5.0.0 release.. and so I guess I ended up doing a bit more work than was necessary. I'll have a look through the CVS system to see what I missed.

The -o flag is provided by -n, and the maxfontsize= functionality is somewhat unneeded now that font size will auto-adjust if it is too large for the window. Otherwise font size is controlled by the size of the legend boxes, which are in turn controlled by the number of categories to display or from user defined mouse/at= sizing.

As far as putting it into 5.0.1 goes, more testing would certainly be nice before letting it out, as all I know is that it "works for me" ..
Also, as far as I am concerned it is still in flux- I am still working on it, adding a little more functionality and flexibility ... I make no claims to being an expert, so there may very well be things I've unwittingly overlooked or done wrong as well.

I can spin out a small diff from the 5.0.0 version which fixes a couple of important bugs without introducing any of the new features if you prefer. (mouse placement x&y swapped & boxes drawn off center)

Hamish

Glynn_Clements · December 3, 2002, 4:58am

H. Bowman wrote:

> > the new d.legend now in CVS

> Did you intentionally remove the -o and maxfontsize= options, or did
> you base your updated version on old code (i.e. the 5.0.0 release
> version instead of the CVS head)?

I based my version on what was in the Grass 5.0.0 release.. and so I
guess I ended up doing a bit more work than was necessary. I'll have a
look through the CVS system to see what I missed.

OK; looking at the "cvs log" output, it appears that Markus needs some
re-education in the ways of version control

Specifically: *don't* just dump a complete file on top of an existing
file then commit it. Instead, request a diff[1]; if the diff doesn't
apply cleanly, that indicates that you have a conflict.

This is analogous to the way in which CVS itself behaves; when you
commit a file, it creates a diff against the last version which you
retrieved, then tries to apply it against the latest version. If
someone else has changed the same code, it tells you, instead of
blindly discarding the other changes.

[1] Either unified ("diff -u") or context ("diff -c") format. "Plain"
diffs (the default format) are risky, because they will apply without
warning to code which has been substantially changed; they are also
harder to read than unified or context diffs.

The -o flag is provided by -n, and the maxfontsize= functionality is
somewhat unneeded now that font size will auto-adjust if it is too
large for the window. Otherwise font size is controlled by the size of
the legend boxes, which are in turn controlled by the number of
categories to display or from user defined mouse/at= sizing.

Neither -o nor maxfontsize= feature in any "released" version, so we
don't really have to worry about backward compatibility.

Discarding code because it isn't relevant is fine; doing so by
accident isn't[2].

[2] Well, not unless we specifically tell would-be developers that we
reserve the right to discard their code for absolutely no reason, but
that doesn't make particularly good PR

As far as putting it into 5.0.1 goes, more testing would certainly be
nice before letting it out, as all I know is that it "works for me" ..

Also, as far as I am concerned it is still in flux- I am still working
on it, adding a little more functionality and flexibility ... I make
no claims to being an expert, so there may very well be things I've
unwittingly overlooked or done wrong as well.

The preceding sentences disfavour inclusion into 5.0.1.

I can spin out a small diff from the 5.0.0 version which fixes a
couple of important bugs without introducing any of the new features
if you prefer. (mouse placement x&y swapped & boxes drawn off center)

If there are definite bugs with simple fixes, they should be
considered separately from non-trivial improvements.

--
Glynn Clements <glynn.clements@virgin.net>

Markus_Neteler1 · December 3, 2002, 8:56am

On Tue, Dec 03, 2002 at 04:58:15AM +0000, Glynn Clements wrote:

H. Bowman wrote:

> > > the new d.legend now in CVS
>
> > Did you intentionally remove the -o and maxfontsize= options, or did
> > you base your updated version on old code (i.e. the 5.0.0 release
> > version instead of the CVS head)?
>
> I based my version on what was in the Grass 5.0.0 release.. and so I
> guess I ended up doing a bit more work than was necessary. I'll have a
> look through the CVS system to see what I missed.

OK; looking at the "cvs log" output, it appears that Markus needs some
re-education in the ways of version control

Specifically: *don't* just dump a complete file on top of an existing
file then commit it. Instead, request a diff[1]; if the diff doesn't
apply cleanly, that indicates that you have a conflict.

My apologies - I was 100% sure that Hamish is working on the
CVS HEAD (why not?).

Now I realize that even the obvious is not always true.

I have reverted the release_branch sync but left the HEAD
untouched for now.

Sorry,
(perhaps a bit overworked)

Markus

Markus_Neteler1 · December 3, 2002, 9:01am

On Tue, Dec 03, 2002 at 04:58:15AM +0000, Glynn Clements wrote:

H. Bowman wrote:

> > > the new d.legend now in CVS
>
> > Did you intentionally remove the -o and maxfontsize= options, or did
> > you base your updated version on old code (i.e. the 5.0.0 release
> > version instead of the CVS head)?

[...]

Neither -o nor maxfontsize= feature in any "released" version, so we
don't really have to worry about backward compatibility.

Discarding code because it isn't relevant is fine; doing so by
accident isn't[2].

[2] Well, not unless we specifically tell would-be developers that we
reserve the right to discard their code for absolutely no reason, but
that doesn't make particularly good PR

The good thing is that -o and maxfontsize= were *my* improvements

ChangeLog:
2002-08-14 11:07 markus

* main.c: a try for bugfix, -m is still not perfect without -s or
-o

2002-08-14 10:30 markus

* main.c: added maxfontsize parameter to optionally avoid BIG
characters in legend (nice with -o flag)

2002-08-13 18:35 markus

* main.c: added -o flag: Draw entries only for existing categories

Note that Hamish's -n does what -o did.
As he was improving the font scaling, the maxfontsize= hack should not be
necessary any more.

Markus

H_Bowman · December 4, 2002, 5:42am

Hamish:

> > > > the new d.legend now in CVS
> > I based my version on what was in the Grass 5.0.0 release.. and so

Markus:

I was 100% sure that Hamish is working on the CVS HEAD (why not?).

Sorry about all the confusion folks, thought I'd checked to see they were the same before I started. Guess not. My apologies.

Markus:

I have reverted the release_branch sync but left the HEAD
untouched for now.

attached pls find a short diff that fixes three bugs in the 5.0.0 release version. (The diff is against the 5.0.0 source, which I think is now the same as what's in the 5.0 CVS, aside from the versioning and header comments.)

-The "set legend box according to mouse settings" fix is important, as the use-mouse option is totally unusable without.

-The G_fatal_error -> exit(-1) was the only bug fix I missed verus the CVS version. (The only other changes were Markus's -o and maxfontsize= enhancements, which I would have changed anyway, luckily).

-The dots_per_line-5 fix makes the output look much better.

**Header comments are now out of sync with the code (11/2002..)

Once 5.0.1 is done, I'll send out my latest and greatest for testing (not finished yet).

Glynn:

Either unified ("diff -u") or context ("diff -c") format. "Plain"
diffs (the default format) are risky, because they will apply without
warning to code which has been substantially changed; they are also
harder to read than unified or context diffs.

Thanks for the tip. Should this be added to the SUBMITTING file?

Glynn:

If there are definite bugs with simple fixes, they should be
considered separately from non-trivial improvements.

see above/below.

regards,
Hamish

(attachments)

5.0.0bugfix.diff (1.14 KB)

Glynn_Clements · December 4, 2002, 10:23pm

H. Bowman wrote:

attached pls find a short diff that fixes three bugs in the 5.0.0
release version. (The diff is against the 5.0.0 source, which I think
is now the same as what's in the 5.0 CVS, aside from the versioning
and header comments.)

The patch doesn't apply to the current CVS head version, but it's
simple enough to make the changes manually.

-The "set legend box according to mouse settings" fix is important,
as the use-mouse option is totally unusable without.

I'll fix this.

  -The G_fatal_error -> exit(-1) was the only bug fix I missed verus
  the CVS version. (The only other changes were Markus's -o and
  maxfontsize= enhancements, which I would have changed anyway,
  luckily).

This appears to be erroneous; what was your reason for this change?

-The dots_per_line-5 fix makes the output look much better.

I think that I see what you're trying to do, but I don't think that
the fix is quite right (all sides should be equal length). I've
applied the attached patch instead.

**Header comments are now out of sync with the code (11/2002..)

Once 5.0.1 is done, I'll send out my latest and greatest for testing
(not finished yet).

Glynn:
> Either unified ("diff -u") or context ("diff -c") format. "Plain"
> diffs (the default format) are risky, because they will apply without
> warning to code which has been substantially changed; they are also
> harder to read than unified or context diffs.

Thanks for the tip. Should this be added to the SUBMITTING file?

Yes; I see that it's already been added (point #25). Although, it
would be worth stating that diffs should be made from the top-level
directory, e.g. "cvs diff src/display/d.leg.thin/main.c"; that way,
the diff will include the pathname rather than just "main.c".

--
Glynn Clements <glynn.clements@virgin.net>

(attachments)

grass-d.leg.thin.diff (1.09 KB)

H_Bowman · December 4, 2002, 11:41pm

> -The G_fatal_error -> exit(-1) was the only bug fix I missed verus
> the CVS version.

This appears to be erroneous; what was your reason for this change?

It is. I was a bit braindead when I looked at the rev 1.6 -> 1.7 diff.
The comedy of errors continues..

> -The dots_per_line-5 fix makes the output look much better.

I think that I see what you're trying to do, but I don't think that
the fix is quite right (all sides should be equal length). I've
applied the attached patch instead.

Yes, that's a better solution.

I see what is in CVS now has the maxfontsize= and -o enhancements. If
they will both be depreciated in the following version, is this really wanted?

Someone might want to back out the src/tcltkgrass/module/d.legend change
as well.

Glynn_Clements · December 5, 2002, 12:49am

H. Bowman wrote:

I see what is in CVS now has the maxfontsize= and -o enhancements.
If they will both be depreciated in the following version, is this
really wanted?

I simply reverted the big overall change, then made the two minor
changes. That way, it's more likely to be feasible to use
"cvs update -j ... -j ..." to merge the bug fixes into the release
version without the larger changes.

In the current situation (i.e. with 5.0.1 overdue), bug fixes should
be separated from more substantial changes.

Someone might want to back out the src/tcltkgrass/module/d.legend change
as well.

Done.

For further updates, it would be best if you could work from the
version in the CVS head, as it differs significantly from the version
in the 5.0.0 release (i.e. patches made against the release version
probably won't apply to the head version). If using CVS is
problematic, you can get individual files from the web interface:

http://freegis.org/cgi-bin/viewcvs.cgi/grass/

--
Glynn Clements <glynn.clements@virgin.net>