[wenzeslaus]

Always use (respect) the color option for the area boundary and apply color table
and other variable color options only to the area fill (as fill_color).

Before

After

Previously, this was only possible by drawing the vector again using another layer (or vector map copy in case color table was used).

Trying to get old behavior with the new code

Unfortunately, color=none which disables drawing boundaries leaves thin white (bg color) lines in between the areas, so the old behavior cannot be exactly reproduced like that.

[wenzeslaus]

@landam Can you please comment both on the whether you agree with the change of behavior and on the correctness of the implementation? And/or suggest another reviewer.

Works for me as expected with the vector color table, RGB column, cat-based random colors, and without any of those but color and fill color specified.

[wenzeslaus]

Added images to the description and highlighted the controversy.

[HuidaeCho]

Hi @wenzeslaus. I'm against this PR. I think, with only one option for an RGB column, the current behavior (same color for both area and boundary) is more expected. Also, leaving thin gaps would not be acceptable in some case. Why don't we split rgb_column (currently, both area and boundary) into area and boundary?

• color and rgb_column for feature (border) color
• fill_color and fill_rgb_column (rgb_column if not given to keep the current behavior) for fill (area) color

Last, with this PR, zcolors seems to be ignored.

[HuidaeCho]

I did a little test. By default, the current d.vect only draws points, lines, areas, and faces from the command line. From the GUI, it draws boundaries as well. I created a red-only RGB column redcol for testing. To show you my data,

d.vect map=wps_hu12

To mimic your problem (no separate single boundary color),

d.erase
d.vect map=wps_hu12 rgb_col=redcol
# or
d.vect map=wps_hu12 rgb_col=redcol col=blue # Woops! col=blue doesn't work?

To overlay boundaries (what you want?),

d.erase
# it's a single run
d.vect map=wps_hu12 type=area,boundary rgb_col=redcol col=blue

In d.vect, area borders are drawn by display_area(), but boundaries by display_lines(). So, even now, you don't need to run d.vect twice to achieve what you want (single-color boundaries over RGB-filled areas) if I'm not missing anything.

From the GUI, it just works because boundary is drawn by default using the color option.

Now, do we need variable colors for area borders (separate from fill colors) or boundaries (new)? That's a different story.

[HuidaeCho]

@wenzeslaus Is this PR still relevant? I cannot reproduce this issue. From the NC sample mapset,

g.copy vect=boundary_county,tmp --o
v.db.addcolumn map=tmp col="rgb_color varchar(50)"
v.db.update tmp col=rgb_color querycol="abs(random()%255) || ':' || abs(random()%255) || ':' || abs(random()%255)"
d.mon start=wx0
g.region vect=tmp
d.vect map=tmp rgb_column=rgb_color color=black type=area,boundary

From the GUI, check boundary (default).

[wenzeslaus]

I was concerned with color table meaning the color rules saved with the map ($MAPSET_PATH/vector/country_boundaries/colr) which I created with:

v.colors map=country_boundaries color=inferno

It still seems to behave the same when I use simple d.vect without checking the boundary for type (default in GUI and CLI), color option is not used. Colors of boundaries and fill are the same. When I check it (d.vect...type=point,boundary,area...) I indeed get the desired result. Is this related to the changes you made?

The behavior seems strange since the boundaries are part of areas in GRASS and they are rendered with type=area already (test e.g. by setting line width high). It just happens that type=boundary is rendered after areas, so the originally rendered boundaries get replaced, or is there some smartness there now? How I'm going to explain this to someone?

[wenzeslaus]

@HuidaeCho Looking at comments here again, it seems your first and second test is the same, in both cases you ended up adding boundary, i.e., type=area,boundary, so I don't think anything changed. I think your suggestion is a good workaround for the original problem, but since the boundary rendering is already part of area rendering, I don't think this is resolved.

One use case for this would be the world map we show when GRASS GIS starts the first time. Using a workaround there would be okay-ish temporarily, but we don't want to keep using a workaround there. (Using RGB column there would be fine too in this case possibly lowering priority of this PR/problem.)

So, I think this PR (or another fix) is still relevant.

[HuidaeCho]

I was concerned with color table meaning the color rules saved with the map ($MAPSET_PATH/vector/country_boundaries/colr) which I created with:

v.colors map=country_boundaries color=inferno

It still seems to behave the same when I use simple d.vect without checking the boundary for type (default in GUI and CLI), color option is not used. Colors of boundaries and fill are the same. When I check it (d.vect...type=point,boundary,area...) I indeed get the desired result. Is this related to the changes you made?

The behavior seems strange since the boundaries are part of areas in GRASS and they are rendered with type=area already (test e.g. by setting line width high). It just happens that type=boundary is rendered after areas, so the originally rendered boundaries get replaced, or is there some smartness there now? How I'm going to explain this to someone?

I see. You were not talking about the rgb_column= and color= options of d.vect. Sorry for the confusion. Still, this PR changes the behavior of color= not only with v.colors, but also with rgb_column=. With this PR without a color table,

g.copy vect=boundary_county,tmp --o
v.db.addcolumn map=tmp col="rgb_color varchar(50)"
v.db.update tmp col=rgb_color querycol="abs(random()%255) || ':' || abs(random()%255) || ':' || abs(random()%255)"

d.vect map=tmp rgb_column=rgb_color type=area

doesn't draw the area-color boundaries anymore. It uses the default solid color now and we lose the area-color boundary feature, which I believe we have to keep supporting.

d.vect map=tmp rgb_column=rgb_color color=none type=area

behaves consistently with your v.colors example and leaves gaps, but I would say that is a feature because you explicitly requested no boundaries.

Let me summarize the current behavior without this PR.

v.colors map=country_boundaries color=inferno
d.vect map=country_boundaries       type=area  # draws the area-color boundaries
d.vect map=tmp rgb_column=rgb_color type=area  # draws the area-color boundaries

d.vect map=country_boundaries       color=blue type=area  # draws the area-color boundaries and ignores color=blue
d.vect map=tmp rgb_column=rgb_color color=blue type=area  # draws the area-color boundaries and ignores color=blue

v.colors -r map=country_boundaries 
d.vect map=country_boundaries type=area             # draws grey areas and black boundaries
d.vect map=country_boundaries color=blue type=area  # draws grey areas and blue boundaries

So the only time boundaries are drawn in their own single color is when areas are not assigned any colors either though rgb_column= or v.colors (custom_rgb in the code). That actually makes sense because without any boundary color and with a single area color, it would be just a single-color union polygon of all the features with no boundaries in-between. The last two behaviors are just necessary, but the other ones with either rgb_column= or v.colors, you do have area colors and by default boundaries use the same area color. Yes, boundaries are still drawn not because they are requested, but because we should fill the gaps. I would say this is a feature. If this PR is committed, not only we will lose this feature, but also it will change the current behavior of rgb_column= with column=.

What you need is again type=area,boundary with no code changes.

d.vect map=country_boundaries color=blue type=area,boundary

This is not a temporary workaround. It uses a feature of d.vect. Boundary drawing is separate from area drawing.

In the end, what would be the point of type=boundary if boundaries were drawn together with areas when type=boundary was not requested?

[HuidaeCho]

(test e.g. by setting line width high

I would say that is a bug. d.vect should not draw thick boundaries when no type=boundary is specified. I mean for type=area and maybe face only.

width_scale, default_width and nrec_width from area.c and lines 190-196.

Lines 441-442 in main.c, but then we need it for line features. So, it would be easier to use option checks to avoid thick boundaries for areas only. And move it to shape.c between display_area() and display_line().

[wenzeslaus]

First of all, I'm all for replacing this PR with something better. However, I have some concerns with what you are suggesting which will explain why I though it is only a workaround.

The last two commands in your bigger example miss type=area,boundary if I understand correctly, but assuming I got that right:

Your type=area versus type=area,boundary makes sense, but it seems to go against GRASS vector topological model and intention in d.vect (intention to follow it in the interface). In GRASS topology, an area is centroid and boundary, so one may reasonably expect that type=area includes boundaries too while type=boundary means only boundaries (isn't this the case for the type parameter for vector modules). Nobody wants to draw centroids (usually), so the d.vect doesn't draw them by default. However, it is common to draw areas/polygons/... with a boundary/outline/..., so I think most people would expect an area to be drawn including its boundary. This seems exactly like the intention in d.vect. Boundaries are rendered together with the fill and, perhaps most importantly, boundary is not included in defaults for the type parameter because it is already included in area.

In this perspective, the width behavior makes perfect sense. This PR works with this understanding too. It changes the d.vect behavior so that it assumes not only you don't want centroid to be rendered by default, but newly also assumes that you want boundaries rendered in a constant color.

Your approach seems to say that area for d.vect means skip not only centroids but also boundaries and render only what d.vect calls fill. In that case, I would say, boundary needs to be added to the defaults for type to achieve the expected behavior (and the boundaries with type=area need to ignore width and possibly render only sometimes). However, I have concerns on how this fits with what other modules do the with type parameter (which is perhaps less important because usefulness and intuitiveness may not always mean absolute consistency) and how d.vect is implemented and how it behaves, see, e.g., the following images where lines drawn by boundary and by area follow different rules.

Both figures: Same world map with color table as above, but with type=area,boundary and width=11 and on top of it another d.vect of the same data but without color table, with color set to white, and with fill_color=none.

# bottom
d.vect map=country_boundaries@with_color_table type=point,line,boundary,area,face color=0:29:57:255 width=11
# top
d.vect map=country_boundaries@no_color_table color=255:255:255:255 fill_color=none width=1

[HuidaeCho]

I found this sentence from the d.vect manual:

By default d.vect areas are filled with fill_color and outlined with color. Area outlines can be suppressed with

d.vect map=vector_map color=none

and areas can be made transparent with

d.vect map=vector_map fill_color=none

I think you're right that the original intention was to draw boundaries with areas by default. That's interesting. I always assumed based on the behavior of d.vect that, at least for display purposes, areas are only interiors not including their boundaries. Anyway, if that is the case, this PR actually fixes the same-color bug. Now I want that bug back as a new feature then. Adding a new flag -b (Use area color for boundaries) would be great. Maybe, in a separate PR once you merge it.

[HuidaeCho]

the following images where lines drawn by boundary and by area follow different rules.

Just curious.. Is it intended?

[ninsbl]

Bumping up milestone as 8.0.1 is due in two days, while this has not been part of RC1 and there has not been activity for some time.

[echoix]

Solved conflicts

[echoix]

I asked an AI agent (Claude Code) to read the discussion, reproduce the issue on current main, and compare the rendering with and without this PR. Below are its findings and a summary of them.

The change does what the description says, but it also changes the default output of d.vect for every variable-color case. I think the maintainers need to decide that part before this can merge.

What the PR does

Area boundaries are always drawn with color. RGB column, color table, -c and zcolor now affect only the fill.

Reproduction and verification

The agent reproduced the bug on main: with a variable color, color= is ignored for area boundaries (case 1 below). The PR fixes that (cases 1, 5, 6, 8).

The same change also has two side effects:

• Case 4: rgb_column with fill_color=none used to draw per-feature colored outlines. After the PR, the outlines are all color, so that output can no longer be produced. As far as the agent could tell, no other d.vect option gives it, but it did not test every combination.
• Cases 2 and 7: with the default color, maps that used to render as seamless fills now get thin dark seams between areas.

What the agent tested (10 cases, main vs this PR)

Setup:

• main at 6a479c7, built with CMake.
• The agent applied this PR's one-hunk diff to display/d.vect/area.c on top of main. It did not check out the PR branch itself.
• Cairo driver, 300x300 px.
• A 2x2 grid of adjacent squares with an RGB column.
• A copy of the grid with a v.colors use=cat color=viridis color table.
• A 3D map of four separated squares at different z, for zcolor.
• All cases use type=area.

#	d.vect options	main	This PR
1	rgb_column=rgb color=black	Boundaries take the area color, so color=black is ignored (the reported bug).	Black boundaries.
2	rgb_column=rgb (default color)	Seamless fills.	Thin dark seams between areas.
3	rgb_column=rgb color=none	Seamless fills.	Same as main.
4	rgb_column=rgb fill_color=none	Per-feature colored outlines, no fill.	Plain color outlines, no fill. The colors are lost.
5	rgb_column=rgb color=black fill_color=none	Colored outlines (color=black is ignored).	Black outlines.
6	color table, color=black	Boundaries in the area color.	Black boundaries.
7	color table, default color	Seamless fills.	Thin dark seams.
8	-c color=black	Boundaries in the area color.	Black boundaries.
9	zcolor=viridis color=black (3D map)	Colored fill, boundaries in the area color.	Colored fill, black boundaries. zcolor still works for the fill.
10	color=blue fill_color=yellow (no variable color)	Yellow fill, blue boundaries.	Identical to main.

A side-by-side image of these renders is attached.

Not tested:

• wxGUI.
• width / width_column interaction.
• Maps with islands.
• Combining rgb_column with a color table.

What is left before merging

• CI. Checks were still queued or running when the agent looked.
• Documentation. d.vect.md should describe the new behavior, in particular that variable colors no longer color the boundaries.
• Comment fix in area.c. The new comment is truncated ("... RGB col...").
• Test. There is none for this behavior. Rendering tests look hard here, so a manual test plan may be enough.

What is left to decide

• Is changing the default acceptable? HuidaeCho's objection (2021-03-02) is that area-colored boundaries are a feature, because they fill the gaps between neighbouring areas. wenzeslaus' reading is that the manual says areas are "outlined with color", so the old behavior is a bug. Neither side has conceded the point.
• If the old behavior should stay available, the options are:
  • Keep the PR as is and add an opt-in for area-colored boundaries, for example the -b flag HuidaeCho suggested. That would also bring back case 4, but only if the flag works together with fill_color=none.
  • Keep the old default and add an opt-in for fixed-color boundaries.
  • Split rgb_column into fill and border columns, as HuidaeCho proposed on 2020-12-31.
• Case 4 needs to be part of that decision. Right now it silently stops working.

Opinion

Not ready to merge as is. The fix is correct for the reported problem, but it is a behavior change, not just a bug fix, and it removes one output that currently works (case 4). I'd merge it only after the maintainers agree on the default and on how people get the old rendering back, or after it is reworked as an opt-in.