[patch] Text-halo-color and text-halo-radius Properties are not applied within area selectors

[skorasaurus]

I have been attempting to style the names of a few features using an eval selector.

For example, I have 10 or so features that include small_area : true; as follows:

area[power=generator]:closed {
    color: black;
    width: 2;
    fill-color: #444444;
    fill-opacity: 0.6;
    small_area : true;}

area[eval(prop(small_area))]:closed {
    text-color: black;
    font-size: 100;
    text: name;
    text-halo-color: white;
    text-halo-radius: 20;
    text-position: center;
}

text-halo-color and text-halo-radius are not applied to my selectors although font-size does.
I have attached a simple test case that shows this bug.

[skorasaurus]

TEST CASE

[AlfonZ]

In 8934-1-HaloedTextInAreas.patch​ the halo drawing code is based on one from drawBoxText().

I noticed that if there's halo set, it gets drawn even when the element (node/way/area) is disabled (filtered out or layer is inactive).
8934-2-HaloedTexts.patch​ in addition disables halo drawing and uses inactiveColor for texts (even for non-haloed ones) of disabled elements.

[bastiK]

Hi AlfonZ, the second patch gets rejected when the first is applied. Can you please combine both in one?

As the halo drawing code is now at 3 places, it might be a good idea to put it in a method...

[AlfonZ]

The second patch is already combined.
I'll look at refactoring tomorrow.

[AlfonZ]

Patch to be applied after patch#2​.
Moved halo drawing from drawArea, drawBoxText and drawTextOnPath to displayText.

[AlfonZ]

Combined patch#2​ and patch#3​.
Moved halo drawing from drawArea, drawBoxText and drawTextOnPath to displayText.
Doing that, areas now have haloed texts, disabled nodes' haloed texts use inactiveColor and disabled ways' texts use inactiveColor.

[AlfonZ]

Patch to be applied after patch#3​.
Replace multiple selections between String and GlyphVector by converting String to GlyphVector beforehand.

[AlfonZ]

Combined patch#2​, patch#3​ and patch#4​.
Moved halo drawing from drawArea, drawBoxText and drawTextOnPath to displayTexts.
Doing that, areas now have haloed texts, disabled nodes' haloed texts use inactiveColor and disabled ways' texts use inactiveColor.
Avoid multiple selections between String and GlyphVector by converting String to GlyphVector beforehand (used by nodes and areas).

[AlfonZ]

Moved halo drawing from drawArea, drawBoxText and drawTextOnPath to displayText - patch#3​.

It works, but doesn't look that nice. drawTextOnPath's usage complicates it a little bit, because it places text along a path character by character, so doesn't use drawString.

I was thinking about adding extra displayText(String,...) that would convert String to GlyphVector and handing that to displayText(GlyphVector,...) which would do the drawing - patch#4​.
It seems to work, at the expense of replacing drawString by drawGlyphVector. Now it might be even possible to remove surrounding Font defaultFont = g.getFont(); ... g.setFont(defaultFont);.

[bastiK]

Looks clear and tidy with patch 4, but I fear it has an impact on the performance when drawString is replaced by drawGlyphVector. What I would do is have both String s and GlyphVector gv as arguments of a single displayText(...) method and then do some checks
if (s != null) { drawString } else { drawGlyphVector }, if (gv == null) { make gv from s; drawGlyphVector } and so on. Not very elegant, maybe you find something better. :)

[AlfonZ]

Replying to bastiK:

What I would do is have both String s and GlyphVector gv as arguments of a single displayText(...) method and then do some checks
if (s != null) { drawString } else { drawGlyphVector }, if (gv == null) { make gv from s; drawGlyphVector } and so on. Not very elegant, maybe you find something better. :)

Do you mean something like in patch#3​?

Looks clear and tidy with patch 4, but I fear it has an impact on the performance when drawString is replaced by drawGlyphVector.

I've done some performance testing, using timer in StyledMapRenderer.render. Dataset consisted of (200 nonhaloed node texts, 200 haloed node texts, 200 nonhaloed line texts, 200 haloed line texts, 200 nonhaloed area texts, 200 haloed area texts).

• without patch
  • normal
    PAINTING TOOK 3063 [PHASE1 took 16] (at scale 11102.581343440308)
PAINTING TOOK 3015 [PHASE1 took 15] (at scale 11102.581343440308)
  • elements disabled (filtered out or layer inactive)
    PAINTING TOOK 3000 [PHASE1 took 15] (at scale 11102.581343440308)
PAINTING TOOK 2906 [PHASE1 took 15] (at scale 11102.557091305049)

• patch#3 (unified halo drawing, use drawString)
  • normal
    PAINTING TOOK 3985 [PHASE1 took 16] (at scale 11343.941807424417)
PAINTING TOOK 3953 [PHASE1 took 15] (at scale 11343.941807424417)
  • elements disabled
    PAINTING TOOK 1453 [PHASE1 took 16] (at scale 11343.92767355973)
PAINTING TOOK 1406 [PHASE1 took 0] (at scale 11343.919695825709)

• patch#4 (unified halo drawing, use drawGlyphVector)
  • normal
    PAINTING TOOK 4031 [PHASE1 took 16] (at scale 10985.71206614267)
PAINTING TOOK 3922 [PHASE1 took 15] (at scale 10985.71206614267)
  • elements disabled
    PAINTING TOOK 1453 [PHASE1 took 16] (at scale 10985.71206614267)
PAINTING TOOK 1421 [PHASE1 took 16] (at scale 10985.712066142663)

My conclusion:
Increased times in normal are due to newly drawn halos on areas' texts.
Decreased times in disabled are due to disabled halo drawing in disabled node texts and line texts.
Replacing drawString by drawGlyphVector doesn't impact performance.

[bastiK]

Replying to AlfonZ:

Replying to bastiK:

What I would do is have both String s and GlyphVector gv as arguments of a single displayText(...) method and then do some checks
if (s != null) { drawString } else { drawGlyphVector }, if (gv == null) { make gv from s; drawGlyphVector } and so on. Not very elegant, maybe you find something better. :)

Do you mean something like in patch#3​?

Yes, exactly.

Looks clear and tidy with patch 4, but I fear it has an impact on the performance when drawString is replaced by drawGlyphVector.

I've done some performance testing, using timer in StyledMapRenderer.render. Dataset consisted of (200 nonhaloed node texts, 200 haloed node texts, 200 nonhaloed line texts, 200 haloed line texts, 200 nonhaloed area texts, 200 haloed area texts).

• without patch
  • normal
    PAINTING TOOK 3063 [PHASE1 took 16] (at scale 11102.581343440308)
PAINTING TOOK 3015 [PHASE1 took 15] (at scale 11102.581343440308)
  • elements disabled (filtered out or layer inactive)
    PAINTING TOOK 3000 [PHASE1 took 15] (at scale 11102.581343440308)
PAINTING TOOK 2906 [PHASE1 took 15] (at scale 11102.557091305049)

• patch#3 (unified halo drawing, use drawString)
  • normal
    PAINTING TOOK 3985 [PHASE1 took 16] (at scale 11343.941807424417)
PAINTING TOOK 3953 [PHASE1 took 15] (at scale 11343.941807424417)
  • elements disabled
    PAINTING TOOK 1453 [PHASE1 took 16] (at scale 11343.92767355973)
PAINTING TOOK 1406 [PHASE1 took 0] (at scale 11343.919695825709)

• patch#4 (unified halo drawing, use drawGlyphVector)
  • normal
    PAINTING TOOK 4031 [PHASE1 took 16] (at scale 10985.71206614267)
PAINTING TOOK 3922 [PHASE1 took 15] (at scale 10985.71206614267)
  • elements disabled
    PAINTING TOOK 1453 [PHASE1 took 16] (at scale 10985.71206614267)
PAINTING TOOK 1421 [PHASE1 took 16] (at scale 10985.712066142663)

My conclusion:
Increased times in normal are due to newly drawn halos on areas' texts.
Decreased times in disabled are due to disabled halo drawing in disabled node texts and line texts.
Replacing drawString by drawGlyphVector doesn't impact performance.

I think you are right that there is no performance difference in drawString and createGlyphVector; drawGlyphVector.

However, there is one problem I wasn't aware of until now. From the javadoc of createGlyphVector:

This means that this method is not useful for some scripts, such as Arabic, Hebrew, Thai, and Indic, that require reordering, shaping, or ligature substitution.

So for example Arabic text is written in reverse order and without ligatures. While the problem is currently visible for text with halo and text along a line, the default JOSM style does not use these features, so it is possible to work without being affected by this bug.

Therefore I think we should stick to patch3b for the moment. (Will be applied when stabilization is over.)

[bastiK]

In 6122/josm:

fixed #8934 - Text-halo-color and text-halo-radius Properties are not applied within area selectors (patch by AlfonZ)