Opened 10 years ago

Closed 10 years ago

#1446 closed enhancement (fixed)

Merge areaSymbol and symbol, areaText and Text (patch included)

Reported by: R2D2_C3PO Owned by: osm@…
Priority: major Milestone:
Component: osmarender Version:
Keywords: Cc:

Description

Osmarender currently has a lot of dupplicate rules, because one is needed for nodes and one for ways. This patch changes the or/p behaviour:

<symbol> works for both ways and nodes. <areaSymbol> is no longer needed but kept for compatibility with old stylefiles. <text> keeps its old meaning (labeling streets, etc). <areaText> now works on nodes too. <circle> works on both nodes and ways.

A bug in orp-select.pm is fixed, so it does not fail on e="node|way".

The current z17 stylesheet is converted to the new syntax. Also some improvements in rendering are done as a result (same rendering of amenity=theatre for ways and nodes).

xslt still needs changes.

Attachments (6)

ways_and_nodes.diff (39.3 KB) - added by R2D2_C3PO 10 years ago.
with xslt
ways_and_nodes2.diff (70.4 KB) - added by R2D2_C3PO 10 years ago.
circle.diff (1.2 KB) - added by R2D2_C3PO 10 years ago.
Fixes circles in areas.
z17-basic.diff (39.1 KB) - added by R2D2_C3PO 10 years ago.
z17-extended.diff (42.4 KB) - added by R2D2_C3PO 10 years ago.
Better rendering of captions (moved upwards when a housenumber is shown, same color for node and way), other small fixes.
z12-16.diff (56.6 KB) - added by R2D2_C3PO 10 years ago.

Download all attachments as: .zip

Change History (18)

Changed 10 years ago by R2D2_C3PO

Attachment: ways_and_nodes.diff added

with xslt

comment:1 Changed 10 years ago by R2D2_C3PO

updated to work with xslt

comment:2 Changed 10 years ago by Knut Arne Bjørndal

Just a small thing: If there should be one instruction to draw text for points and areas I'd rather call it something like pointText instead of areaText.

Or maybe the text instruction should be renamed pathText and the other one could be text, that would probably be the most logical, although it would require changes to all osmarender stylesheets simultaneously, which we shouldn't count on being possible.

comment:3 in reply to:  2 ; Changed 10 years ago by Knut Arne Bjørndal

Replying to bob@cakebox.net:

Just a small thing: If there should be one instruction to draw text for points and areas I'd rather call it something like pointText instead of areaText.

Or maybe the text instruction should be renamed pathText and the other one could be text, that would probably be the most logical, although it would require changes to all osmarender stylesheets simultaneously, which we shouldn't count on being possible.

One way to change these without breaking backwards compatibility would be to call the methods pointText and pathText, add a wrapper called text that calls pathText and one called areaText that calls pointText.

That way old stylesheets keep working, and the proper function names get suitable names that better explain what they do.

comment:4 in reply to:  3 ; Changed 10 years ago by R2D2_C3PO

Replying to bob@cakebox.net:

One way to change these without breaking backwards compatibility would be to call the methods pointText and pathText, add a wrapper called text that calls pathText and one called areaText that calls pointText.

Calling it point text is as bad as calling it areaText, as it works on both points and areas.

Here's summary what works where:

Before:
text: ways (places label along path, e.g. for labeling streets), nodes (places text at node's position)
areaText: ways (places label at the center of the way)

After:
text: ways, nodes (unchanged)
areaText: ways (center of area), nodes (just like text)

So it would be best to rename text to pathText (because it's no longer needed for nodes) and rename areaText to text. However this will lead to confusion and all styles have to be updated at the same time.

comment:5 in reply to:  4 Changed 10 years ago by Knut Arne Bjørndal

Replying to R2D2_C3PO:

Replying to bob@cakebox.net:

One way to change these without breaking backwards compatibility would be to call the methods pointText and pathText, add a wrapper called text that calls pathText and one called areaText that calls pointText.

Calling it point text is as bad as calling it areaText, as it works on both points and areas.

No, because areaText just computes a point and puts text there, so it's not too wrong to call that pointText.

comment:6 Changed 10 years ago by R2D2_C3PO

How about this: areaText is called label because you usually use it to put a label somewhere. And we keep the old name as an alias for some time to maintain compatibility?

Changed 10 years ago by R2D2_C3PO

Attachment: ways_and_nodes2.diff added

comment:7 Changed 10 years ago by R2D2_C3PO

Added a patch that uses pathText and caption (because most styles are already named caption-...).

comment:8 Changed 10 years ago by osm@…

Status: newassigned

I'll apply this patch this weekend. If you could regenerate the patch against the current state in svn I would be glad, otherwise I'll have to handle the conflicts myself.

comment:9 Changed 10 years ago by osm@…

Resolution: fixed
Status: assignedclosed

The changes to the code and the changes to the z17 stylesheet are in along with some whitespace cleanup. Further cleanups of the stylesheets are of course welcome.

Changed 10 years ago by R2D2_C3PO

Attachment: circle.diff added

Fixes circles in areas.

comment:10 Changed 10 years ago by R2D2_C3PO

Resolution: fixed
Status: closedreopened

You missed an important part of my patch. The drawCircle function in XSLT needs 2 more parameters now, as otherwise the output will be like this:

<circle cx="NaN" cy="NaN" r="0.9px" ...

which is wrong SVG and crashes Inkscape. A diff with only the relevant patch is attached.

comment:11 Changed 10 years ago by R2D2_C3PO

There are also a few errors in the stylesheet:

amenity=parking was present twice. Housenumbers were rendered incorrectly (circle moved far to the right for areas)

Patch is attached.

And some things I'm unsure about:

Is it really intended, that for historic=museum an icon is displayed when it is a node but only an area without an icon is displayed when it is an area? (lines 2486-2496)

I think all place names should be rendered the same no matter if it is a node or a way (line 2860ff.)

I'll add an patch with these changes later and you can decide if you want to apply it.

Changed 10 years ago by R2D2_C3PO

Attachment: z17-basic.diff added

Changed 10 years ago by R2D2_C3PO

Attachment: z17-extended.diff added

Better rendering of captions (moved upwards when a housenumber is shown, same color for node and way), other small fixes.

Changed 10 years ago by R2D2_C3PO

Attachment: z12-16.diff added

comment:12 Changed 10 years ago by osm@…

Resolution: fixed
Status: reopenedclosed

Ups. Sorry. The problem was that there were conflicts all over the place and I had to apply the patch by hand. I guess I missed the change to the circles. Now I applied circle.diff and everything sould be fine again. I also applied z17-extended.diff and z12-16.diff.

Note: See TracTickets for help on using tickets.